[FFmpeg-devel] [PATCH] Do not override pix_fmt in rawdec.c

[Michael Niedermayer]

On Wed, Jun 09, 2010 at 04:41:39PM +0200, Tomas H?rdin wrote:
> On Wed, 2010-06-09 at 15:38 +0200, Michael Niedermayer wrote:
> > On Wed, Jun 09, 2010 at 11:38:14AM +0200, Tomas H?rdin wrote:
> > > On Mon, 2010-06-07 at 22:23 +0200, Michael Niedermayer wrote:
> > > > >  avcodec.h |    5 ++++-
> > > > >  rawdec.c  |    2 ++
> > > > >  2 files changed, 6 insertions(+), 1 deletion(-)
> > > > > d44a164fed7c1a5cd69f362d521ef66aad6f42b2  rawdec_pix_fmt2.patch
> > > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > > index a6f6b04..d0aa162 100644
> > > > > --- a/libavcodec/avcodec.h
> > > > > +++ b/libavcodec/avcodec.h
> > > > > @@ -1120,8 +1120,11 @@ typedef struct AVCodecContext {
> > > > >  
> > > > >      /**
> > > > >       * Pixel format, see PIX_FMT_xxx.
> > > > > +     * May be set by the demuxer, especially when demuxing rawvideo.
> > > > > +     * Some demuxers use bits_per_coded_sample or codec_tag for this purpose instead.
> > > > 
> > > > > +     * Some codecs may override this value. The rawvideo decoder will not.
> > > > 
> > > > i really would like to avoid adding special cases
> > > > 
> > > > 
> > > > >       * - encoding: Set by user.
> > > > > -     * - decoding: Set by libavcodec.
> > > > > +     * - decoding: Set by user and/or libavcodec.
> > > > 
> > > > i would say
> > > > Set by user if known, overridden by libavcodec if known
> > > 
> > > OK. I changed the explanation a bit as well. Patch attached. I also
> > > reran the regtests against latest SVN, which worked fine.
> > > 
> > > For future reference, I searched for all files where both
> > > CODEC_ID_RAWVIDEO and bits_per_coded_sample are used in a video context.
> > > This in order to hopefully simplify rawdec.c later.
> > > 
> > > AVI: avidec.c, riff.c
> > > MOV: mov.c, movenc.c
> > > Others: utils.c, nsvdec.c, vwfcap.c, rawdec.c
> > > Soon: mxf.c, mxfdec.c
> > > 
> > > /Tomas
> > 
> > >  avcodec.h |    4 +++-
> > >  rawdec.c  |    2 ++
> > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > > 08923a4cccf43a6a6853a1794b17fef2dffa623e  rawdec_pix_fmt3.patch
> > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > index 5e77fb3..ffc2e07 100644
> > > --- a/libavcodec/avcodec.h
> > > +++ b/libavcodec/avcodec.h
> > > @@ -1121,8 +1121,10 @@ typedef struct AVCodecContext {
> > >  
> > >      /**
> > >       * Pixel format, see PIX_FMT_xxx.
> > > +     * May be set by the demuxer if known from headers.
> > > +     * May be overriden by the decoder if it knows better.
> > >       * - encoding: Set by user.
> > > -     * - decoding: Set by libavcodec.
> > > +     * - decoding: Set by user if known, overridden by libavcodec if known
> > >       */
> > >      enum PixelFormat pix_fmt;
> > 
> > ok
> > 
> > 
> > >  
> > > diff --git a/libavcodec/rawdec.c b/libavcodec/rawdec.c
> > > index ab13bdc..69b72d1 100644
> > > --- a/libavcodec/rawdec.c
> > > +++ b/libavcodec/rawdec.c
> > > @@ -73,12 +73,14 @@ static av_cold int raw_init_decoder(AVCodecContext *avctx)
> > >  {
> > >      RawVideoContext *context = avctx->priv_data;
> > >  
> > > +    if (avctx->pix_fmt == PIX_FMT_NONE) {
> > >      if (avctx->codec_tag == MKTAG('r','a','w',' '))
> > >          avctx->pix_fmt = find_pix_fmt(pix_fmt_bps_mov, avctx->bits_per_coded_sample);
> > >      else if (avctx->codec_tag)
> > >          avctx->pix_fmt = find_pix_fmt(ff_raw_pix_fmt_tags, avctx->codec_tag);
> > >      else if (avctx->bits_per_coded_sample)
> > >          avctx->pix_fmt = find_pix_fmt(pix_fmt_bps_avi, avctx->bits_per_coded_sample);
> > > +    }
> > 
> > i think this skips too much, considering the new documentation
> 
> Fair enough. I made the attached patch only skip the dubious
> bits_per_coded_sample step instead since that is only used by demuxers
> that don't know pix_fmt. Re-ran tests as well.
> 
> /Tomas

>  avcodec.h |    4 +++-
>  rawdec.c  |    2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 27e1724487fdb69d923dd572dba2b89bd63ac5da  rawdec_pix_fmt4.patch

looks ok if it works

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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- 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/20100609/d2175ae3/attachment.pgp>