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

[Tomas Härdin]

On Wed, 2010-06-09 at 19:44 +0200, Michael Niedermayer wrote:
> 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

Works fine and I can't find any cases where this restriction would cause
any problems. Also, test pass so: applied.

I'll post further rawvideo suggestions in the rawenc thread since that
is fairly related to this one.

/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100610/bd3f4f48/attachment.pgp>