On Wed, 28 Oct 2015 16:05:49 -0400 "Ronald S. Bultje" <rsbul...@gmail.com> wrote:
> Hi, > > On Wed, Oct 28, 2015 at 3:44 PM, wm4 <nfx...@googlemail.com> wrote: > > > On Wed, 28 Oct 2015 12:21:05 -0400 > > "Ronald S. Bultje" <rsbul...@gmail.com> wrote: > > > > > --- > > > libavcodec/vp9_parser.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/libavcodec/vp9_parser.c b/libavcodec/vp9_parser.c > > > index 0437097..6713850 100644 > > > --- a/libavcodec/vp9_parser.c > > > +++ b/libavcodec/vp9_parser.c > > > @@ -64,7 +64,7 @@ static int parse_frame(AVCodecParserContext *ctx, > > const uint8_t *buf, int size) > > > if (ctx->pts == AV_NOPTS_VALUE) > > > ctx->pts = s->pts; > > > s->pts = AV_NOPTS_VALUE; > > > - } else { > > > + } else if (ctx->pts != AV_NOPTS_VALUE) { > > > s->pts = ctx->pts; > > > ctx->pts = AV_NOPTS_VALUE; > > > } > > > > I find this a bit questionable. What is it needed for? Wouldn't it > > repeat the previous timestamp if new packets have none? > > > I don't think so. Let's first go through the problem that I'm seeing, maybe > you see alternate solutions. Then I'll explain (very end) why I don't think > this happens. > > So, we're assuming here (this is generally true) that all input to the > parser has timestamps (coming from ivf/webm), and that some frames are > "superframes" which have one invisible (alt-ref) frame (only a reference, > not an actual frame that's ever displayed) packed with the following > visible frame. So each packet in ivf/webm leads to one visible output > frame, but in some cases this requires decoding of multiple (invisible) > references. For frame threading purposes, we split before we send it to the > decoder. > > So what you get is: > - ivf/webm file has packet of size a with timestamp b, calls parser > - parser notices that packet is superframe with 2 frames in it > - we output the first (invisible) frame with no timestamp, and cache the > timestamp of the input packet > - utils.c code calls parser again with the same input data (we told it we > didn't consume any), but the (input) timestamp is now set to nopts > - we output the second (visible) frame with the cached timestamp on the > packet > > This generally makes sense, the webm/ivf indeed assume that the timestamp > only is valid for the visible frame. Invisible frames have no timestamp > associated with them since they're never displayed. > > So, the above code has the issue: what if there's 2 invisible frames packed > in a superframe (followed by the visible frame)? Right now, this happens: > - ivf/webm file has packet of size a with timestamp b, calls parser > - parser notices that packet is superframe with 3 frames in it > - we output the first (invisible) frame with no timestamp, and cache the > timestamp of the input packet > - utils.c code calls parser again with the same input data (we told it we > didn't consume any), but the (input) timestamp is now set to nopts > - we output the second (invisible) frame with no timestamp, and cache the > timestamp of the input packet (which is now set to nopts) > - utils.c code calls parser again with the same input data (we told it we > didn't consume any), but the (input) timestamp is now set to nopts > - we output the third (visible) frame with the cached timestamp on the > packet, which was nopts > > The last 3 are wrong; we should only cache timestamps if there is any to be > cached, we should not override the cached timestamp with a new nopts value, > at least not in this particular case. > > -- > very end > -- > > Ok, so what about your point: can we output the same timestamp twice? I > don't think so, because as soon as we output the cached timestamp, we reset > the value of the cached timestamp to nopts, so if we were to reuse the > cached timestamp, it would be nopts and the output data would have no > timestamp. > > (Hope that wasn't too detailed.) Thanks for the explanations. I didn't know there could be more than 1 super frame, and I see how the new logic works and doesn't duplicate timestamps. Patch looks good with me then. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel