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

[Michael Niedermayer]

On Thu, Aug 21, 2008 at 09:58:33AM +0300, Uoti Urpala wrote:
> 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, 

do you happen to have a link to the mail? I am just curious what the exact
question/discussion was to which i suggested has_b_frames ...


> and lavc of course
> did not support reordering in the decoder.

lavc supported reordering in the decoder since it supported direct rendering
that predates any discussion we have had


> 
> 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

Are you complaining that i improved the API or are you jealous because i
fixed it when mans complaind while i didnt when you did?


> 
> > 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.

this reminds me of the word overkill, void * is a recipe for problems, like
memory leaks and a 64bit double surely can be stored in a opaque int64_t.
of course if you do want a union of double and int64_t then iam not opposed
to approve a patch


> 
> 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

Sounds like a argument against your suggestion.


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080821/29f76bdb/attachment.pgp>