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.)
Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
