[FFmpeg-devel] [PATCH 7/7] Add check in av_write_header() which validates the rawvideo codec tag.

[Michael Niedermayer]

On Wed, Jun 02, 2010 at 05:19:58PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2010-06-01 19:59:55 +0200, Michael Niedermayer encoded:
> > On Tue, Jun 01, 2010 at 07:29:34PM +0200, Stefano Sabatini wrote:
> > > On date Tuesday 2010-06-01 17:53:36 +0200, Michael Niedermayer encoded:
> > > > On Tue, Jun 01, 2010 at 12:39:41PM +0200, Stefano Sabatini wrote:
> > > > > On date Thursday 2010-05-27 21:59:52 +0200, Michael Niedermayer encoded:
> > > > > > On Thu, May 27, 2010 at 12:28:28AM +0200, Stefano Sabatini wrote:
> > > > > > > Make the function fail if there is no codec tag associated to that
> > > > > > > pixel format.
> > > > > > > ---
> > > > > > >  libavformat/utils.c |    9 +++++++++
> > > > > > >  1 files changed, 9 insertions(+), 0 deletions(-)
> > > > > > 
> > > > > > this patch requires as prereqesite avienc/riff to override codec_tag
> > > > > > so its 0 for rgb formats
> > > > > 
> > > > > The problem with the current code is that if
> > > > > avcodec_pix_fmt_to_codec_tag(avctx->pix_fmt) in rawenc.c returns 0,
> > > > > then av_write_header() does:
> > > > > st->codec->codec_tag= av_codec_get_tag(s->oformat->codec_tag, st->codec->codec_id);
> > > > > 
> > > > > which sets the codec tag to the first entry with codec id == RAWVIDEO,
> > > > > which may have nothing to do with the pixel format of the output
> > > > > stream.
> > > > 
> > > > you are confused.
> > > > codec_tag must be 0 for rgb in avi. and the first entry is 0
> > > > if rgb in avi is stored with non 0 codec_tag currently then you
> > > > introduced a very serious bug. And the fix is to override it in
> > > > riff.c as i said multiple times.
> > > 
> > > I have not the faintest idea about what "rgb in avi" is, there are
> > > tons of RGB pixel formats which can fit in rawvideo, also I have no
> > > idea what you mean by "override it in riff.c" and I'm quite tired of
> > > playing at mind-guessing.
> > 
> > what is unclear on
> > rgb in avi must use codec_tag=0 ?
> > the raw rgb format in avi is indicated by the number of bits per pixel
> > not by the fourcc which must be set to 0.
> > 
> > 
> > > 
> > > What the code currently implements is just wrong for both AVI and NUT,
> > > in the case of AVI if the pixel format cannot be mapped to any codec
> > > tag, codec_tag is set to 0,
> > 
> > it must be 0 this is as it should be
> > 
> > 
> > > which when reading it is interpreted as
> > > YUV420P,
> > 
> > if so thats a bug and i would suspect a new one
> 
> Not a new bug, I'm simply using FFmpeg in a way that nobody never did
> (read: weird formats).
> 
> static av_cold int raw_init_decoder(AVCodecContext *avctx)
> {
>     RawVideoContext *context = avctx->priv_data;
> 
>     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);
> 
> find_pix_fmt() returns PIX_FMT_YUV420P when pix_fmt_bps_avi doesn't
> contain an entry corresponding to avctx->bits_per_coded_sample, I'm
> testing with bgr444 and there are no entry corresponding to bps = 12.

bgr444 is 12 bpp ?
besides why did we call it 444, this doesnt seem all that great for a
name?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- 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/20100603/3fbe54ea/attachment.pgp>