[MPlayer-dev-eng] [PATCH] unified timing patch for H264

[Reimar Döffinger]

On Sun, Aug 29, 2010 at 05:46:29PM +0200, Pásztor Szilárd wrote:
> > > +    if (mpi && mpi->type == MP_IMGTYPE_NULL) {
> > > +   *full_frame = 0;
> > > +   mpi = NULL;
> > > +    }
> > > +    if (correct_pts && pts != MP_NOPTS_VALUE
> > > +        && (mpi || sh_video->num_buffered_pts < delay)) {
> > 
> > Huh? Wasn't it exactly this code that was supposed to behave
> > differently for mpi == NULL vs. MP_IMGTYPE_NULL?
> 
> No. The point is to propagate the return value ("got_picture") further, and
> that's done in full_frame as a return value. Here mpi is set to 0 in such a
> case, for the rest of the function to behave the same way as before

So far I agree.

> and
> then checked to buffer PTS only if we have an actual image (except when we
> need the buffer to fill up, which will be flooded out in a few frames' time
> anyway).
>
> So, after this function returns, there's no difference in values at
> all, except that we have a full_frame to indicate if we need to wait one
> frame duration or not, which is used in mplayer.c.

These two contradict each other. It's not possible that at the same time
"there's no difference at all" but at the same time "checked to buffer
PTS only if we have an actual image".
You are adding the
mpi || sh_video->num_buffered_pts < delay
condition.
This changes behaviour for all cases and codecs where mpi is NULL or the
new special type, but the idea was to _only_ change behaviour for fields
in H.264.
Which means this code must behave 100% identical to before unless
it gets a mpi->type == MP_IMGTYPE_NULL. And that it doesn't with your patch.