[FFmpeg-devel] [PATCH] Make nut muxer and demuxer recognize the extended nut tags associated to rawvideo pixel formats

[Michael Niedermayer]

On Sun, Mar 28, 2010 at 07:50:40PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2010-03-23 04:23:01 +0100, Michael Niedermayer encoded:
> > On Tue, Mar 23, 2010 at 01:05:49AM +0100, Stefano Sabatini wrote:
> [...]
> > > > the codec_tag-pix_fmt mapping is in libavcodec/raw*
> > > > add it there where the avi and quicktime mappings already are.
> > > 
> > > I thought that that mapping would be nut-specific, so I fail to see
> > > why it should be implemented in
> > > libavcodec/raw.c:ff_raw_pixelFormatTags.
> > 
> > because thats how its done. And thats how all non raw video decoders
> > set the pix_fmt.
> > the mpeg-ps muxer doesnt set pix_fmt to yuv420p or 422p depending on the
> > mpeg-es headers
> > Iam not interrested in redesigning this, its fine as it is, it works.
> > Find a bug and we can talk but not because you dont like the design
> > and that doesnt mean anyone would like the alternative more.
> 
> I don't care about the design, I just want the thing working, it this
> can be done with the current design I'm all fine.
> 
> > > Also that would introduce the possibility for the AVI/QT muxer to use a
> > > tag which is not valid for it, no?
> > 
> > depends on how you add the tags
> 
> Please elaborate. 
> 
> > > That said, I really don't mind into putting these nut tag definitions
> > > in ff_raw_pixelFormatTags, but that sounds odd/wrong to me.
> > 
> > setting pix_fmt in the mpeg-ps demxuer sounds wrong to me
> > setting pix_fmt for some cases in the demxuer and for some cases in the
> > decoder sounds like something i dont want to have to maintain.
> > so whats left is what we have. The least ugly IMHO but iam sure some
> > will disagree ...
> 
> Now I have this problem:
> libavcodec/raw.c:
>     { PIX_FMT_GRAY8,   MKTAG('G', 'R', 'E', 'Y') },

>     { PIX_FMT_RGB555,  MKTAG('R', 'G', 'B', 15) },
>     { PIX_FMT_BGR555,  MKTAG('B', 'G', 'R', 15) },
>     { PIX_FMT_RGB565,  MKTAG('R', 'G', 'B', 16) },
>     { PIX_FMT_BGR565,  MKTAG('B', 'G', 'R', 16) },
>     { PIX_FMT_RGB565,  MKTAG( 3 ,  0 ,  0 ,  0 ) },

this looks wrong, for which files was that added?


> 
> Now I want to add these entries, which are specific to nut:
>     { PIX_FMT_RGB555LE,  MKTAG('R', 'G', 'B',  15) },
>     { PIX_FMT_BGR555LE,  MKTAG('B', 'G', 'R',  15) },
>     { PIX_FMT_RGB565LE,  MKTAG('R', 'G', 'B',  16) },
>     { PIX_FMT_BGR565LE,  MKTAG('B', 'G', 'R',  16) },
>     { PIX_FMT_RGB555BE,  MKTAG( 15, 'B', 'G', 'R') },
>     { PIX_FMT_BGR555BE,  MKTAG( 15, 'R', 'G', 'B') },
>     { PIX_FMT_RGB565BE,  MKTAG( 16, 'B', 'G', 'R') },
>     { PIX_FMT_BGR565BE,  MKTAG( 16, 'R', 'G', 'B') },
> 
> as the RGB5X5 is NE, in a BE machine we'll end-up with a non valid
> tag for nut, as I'm supposed to append these entries.
> 

> In general there is no guarantee that the tag<->pix_fmt mapping in
> libavcodec/raw.c won't conflict with the mapping used by nut (or other
> formats as well).

nut should use tags compatible with avi, libavcodec/raw.c supports avi.


> 
> Also adding such mapping in ff_raw_pixelFormatTags will result into
> making the other formats use tags which are reserved to nut, so I

and otherwise they would not have had any tag and would have failed too


> still fail to see how I can fix this problem without to use a nut
> specific table (check attached patch series - yet untested but posted
> just to show the approach).

[...]

>  nut.c    |   21 +++++++++++++++++++++
>  nut.h    |    3 +++
>  nutdec.c |    8 +++++++-
>  nutenc.c |   11 ++++++++---
>  4 files changed, 39 insertions(+), 4 deletions(-)
> a34c797c8e633a1ca0ab9e347da67351675b89a2  0004-Define-the-ff_nut_raw_video_pixel_format_tags-table-.patch
> >From b39ed7e7174f9467e70a63359c3c5f26074c4e25 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> Date: Sun, 28 Mar 2010 11:39:52 +0200
> Subject: [PATCH 4/5] Define the ff_nut_raw_video_pixel_format_tags table and
>  make the muxer and demuxer use it.

you cannot just handle one format completely different from the others.
this is going to lead to more bugs than this might fix
and again, i dont see the problem you are trying to fix.

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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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/20100330/3a04639f/attachment.pgp>