[FFmpeg-cvslog] r19091 - trunk/libavcodec/4xm.c

[Michael Niedermayer]

On Sat, Jun 06, 2009 at 01:49:35AM -0700, Baptiste Coudurier wrote:
> Baptiste Coudurier wrote:
> > On 6/5/2009 2:25 PM, Michael Niedermayer wrote:
> >> On Fri, Jun 05, 2009 at 02:09:05PM -0700, Baptiste Coudurier wrote:
> >>> On 6/5/2009 1:10 PM, Michael Niedermayer wrote:
> >>>> On Fri, Jun 05, 2009 at 10:12:14AM +0200, bcoudurier wrote:
> >>>>> Author: bcoudurier
> >>>>> Date: Fri Jun  5 10:12:14 2009
> >>>>> New Revision: 19091
> >>>>>
> >>>>> Log:
> >>>>> 4xm decoder uses get_buffer, set CODEC_CAP_DR1
> >>>> i think you broke several decoders with these changes, no i dont
> >>>> know which but
> >>>> CODEC_CAP_DR1 does not just mean the codec uses get_buffer()
> >>>> but it means it works with any get_buffer() implementation while
> >>>> lack of CODEC_CAP_DR1 means it requires the libavcodec internal get_buffer()
> >>> Interesting, I did misunderstand the Doxygen then:
> >>> /**
> >>>  * Codec uses get_buffer() for allocating buffers.
> >>>  * direct rendering method 1
> >>>  */
> >>> #define CODEC_CAP_DR1             0x0002
> >>>
> >>>> one case (that applies to 4xm IIRC) is that linesize == width*bpp is required
> >>>> by some decoders
> >>> Well, user can override the function in any case, so decoder must not
> >>> use get_buffer in AVCodecContext IMHO.
> >> hmm, the original idea i think was that the user would not override
> >> get_buffer when CODEC_CAP_DR1 is not set but it seems not documented
> >> anywhere, this is of course not good
> >>
> >> we either could fix the docs and call everyone who overrides get_buffer
> >> without a CODEC_CAP_DR1 buggy
> >> or
> >> change decoders to not use (re)get_buffer/release_buffer but the default
> >> code directly
> >>
> >> IMHO, because overriding get_buffer without a set CODEC_CAP_DR1 always was
> >> buggy i think user apps should be fixed that do this
> > 
> >> I mean even if we change codecs to use default_*buffer() directly, user apps
> >> would still fail to work with old libavcodec
> >> thus IMHO this is a bug in the docs not the code
> > 
> > IMHO, codecs should be updated to use avcodec_default_get_buffer
> > directly which will avoid confusion.
> 
> Well, I need to revise my statement. Doing that for old codecs this
> might have weird effects for users already overriding get_buffer,
> example the old way of setting pts used by ffplay, I think many
> applications did that unfortunately and ffplay didn't check for
> CODEC_CAP_DR1 IIRC.

I really think updating the docs is the most feaseable solution, we
did in the past even litterally recommand to use get_buffer() for
pts ...

I would suggest to add a simple
"if CODEC_CAP_DR1 is not set then (re)get_buffer() must call
 avcodec_default_get_buffer() instead of providing buffers allocated by
 some other means"
to the (re)get_buffer doxy


> 
> > [...]
> > 
> > I'll check decoders and revert/modify them on a per case basis once we
> > settled this.
> > 
> 
> Btw, If I understood correctly how this works, here is a patch for 4xm
> decoder which should use reget_buffer.

please elaborate why you think reget_buffer() would be needed

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- 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-cvslog/attachments/20090606/199d6dd1/attachment.pgp>