[FFmpeg-devel] [PATCH] lavc: set frame defaults in internals codec frames

[Michael Niedermayer]

On Tue, May 03, 2011 at 12:03:35AM +0200, Stefano Sabatini wrote:
> On date Monday 2011-05-02 12:29:58 +0200, Michael Niedermayer encoded:
> > On Mon, May 02, 2011 at 10:57:21AM +0200, Stefano Sabatini wrote:
> > > On date Monday 2011-05-02 02:18:11 +0200, Stefano Sabatini encoded:
> > > > This is required, specifically for setting frame->format to -1,
> > > > otherwise it will be set to 0 = PIX_FMT_YUV420P and code reading
> > > > format from the output decoded frame will get misled.
> > > > 
> > > > In particular fix regressions occurring with the pending vsrc_buffer
> > > > patch.
> > > > ---
> > > >  libavcodec/dnxhddec.c    |    1 +
> > > >  libavcodec/dv.c          |    1 +
> > > >  libavcodec/flashsv.c     |    1 +
> > > >  libavcodec/huffyuv.c     |    1 +
> > > >  libavcodec/mjpegdec.c    |    1 +
> > > >  libavcodec/qtrle.c       |    2 ++
> > > >  libavcodec/roqvideodec.c |    2 ++
> > > >  7 files changed, 9 insertions(+), 0 deletions(-)
> > > 
> > > I'm working through the complete fate suite and there a lot more than
> > > these. So my question is, is acceptable to require to use
> > > avcodec_get_frame_defaults() in all codecs?
> > > 
> > > Alternatively we may just set frame.format = -1, but using
> > > avcodec_get_frame_defaults() should be more future-proof.
> > 
> > i agree, its probably a good idea to use avcodec_get_frame_defaults()
> > the patch also LGTM if it works
> > 
> > also note we might need special freeing code if frames ever use
> > string type
> 
> Updated, passes fate when the vsrc_buffer patch is applied.  Possibly
> other codecs may be affected, I'll check, but fixing them would be
> trivial.
> 
> BTW, why the function is named avcodec_GET_frame_defaults rather
> than avcodec_SET_frame_defaults?

because setting frame defaults would be like changing the defaults
(which are constant and cant be changed)
set_frame_to_defaults() .... but then i dont want to involve myself
in these bikeshed discussions ATM

anyway i quickly scrolled through the diff and it should be ok

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

There seems to be only one solution to NIH syndrom, ... a shooting squad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110503/0f3cf3f1/attachment.asc>