[FFmpeg-devel] [PATCH] Generic part of frame multithreading

Uoti Urpala uoti.urpala
Thu Aug 21 08:58:33 CEST 2008


On Wed, 2008-08-20 at 23:37 +0200, Michael Niedermayer wrote:
> On Wed, Aug 20, 2008 at 10:23:31PM +0300, Uoti Urpala wrote:
> > On Wed, 2008-08-20 at 21:00 +0200, Michael Niedermayer wrote:
> > > > mplayer counts frame delay using avctx->has_b_frames, which works for  
> > > > h264 and doesn't work with this. ffplay counts delay by adding pts to  
> > > > frames in get_buffer, which works with this but might not work with  
> > > > h264, because of the frame num gap code. I thought about making avctx- 
> > > >  >delay accurate for decoding too, which should solve all this.
> > > 
> > > mplayer is violating lavc API ...
> > > IIRC ive rejected the use of has_b_frames back then when uoti suggested it
> > 
> > You remember things backwards. The preliminary version of -correct-pts
> > used buffer callbacks to count the number of buffered frames, and it was
> > you who suggested using has_b_frames instead.
> 
> I do remember rejecting the even more broken 
> "callbacks to count the number of buffered frames" and i suspect i told you
> back then that the timestamps should be given to the decoder to reorder them.
> Maybe you rejected this and i then suggested to use has_b_frames, but if my

Your first suggestion was using has_b_frames instead, and lavc of course
did not support reordering in the decoder.

You also implemented reordering with buffer callbacks in ffplay.c
yourself later, and even suggested that buffer callbacks should be a
documented way of implementing such functionality with lavc:
http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2008-March/043661.html

> guess (and this is only a guess) is true it would really be more a
> "least broken solution within the constraints" not a "working solution"

You finally added some reordering support to lavc, but I think the
current API is still lacking.

Using int64_t as the type of an opaque user-set field is unnecessarily
limiting. MPlayer would use doubles, and void * should generally be
available for storing arbitrary data in opaque user fields. I think an
union of int64_t, double and void * would be a practical choice.

There should be some way for the user to know that a particular opaque
field is not going to ever be returned. This would be needed to avoid
memory leaks if storage was allocated for data used with the void *, and
would also have other uses. Maybe the ability to do this in the buffer
free callback would be enough (if you don't want to implement a nicer
API), but even that requires some support from lavc to work reliably.


Even if lavc gets a working reordering API I intend to still keep a
delay-sized reordering buffer as an alternative in MPlayer. IIRC there
are some videos with invalid pts (muxed improperly from AVI etc) and
such a reordering buffer seems like the most robust alternative to get
decent playback. So some way to know the current decoder delay is still
required. Getting information about dropped opaque fields would allow
counting this too.





More information about the ffmpeg-devel mailing list