[FFmpeg-devel] [Patch/RFC] Improved PixelLayout parsing in mxfdec.c

[Tomas Härdin]

On Thu, 2010-06-24 at 18:42 -0700, Baptiste Coudurier wrote:
> On 06/10/2010 06:20 AM, Tomas H?rdin wrote:
> > On Mon, 2010-06-07 at 11:36 +0200, Tomas H?rdin wrote:
> >> On Thu, 2010-06-03 at 15:11 -0700, Baptiste Coudurier wrote:
> >> [snip]
> >>
> >>>
> >>> Thanks for the patch, this is appreciated.
> >>>
> >>> [...]
> >>>
> >>
> >> Revised patch attached. It does not pass regtests since pix_fmt differs
> >> (not setting it makes them pass).
> >
> > After poking around a bit with rawdec and tracking the usages of
> > bits_per_coded_sample I realized that as long as all the previous pixel
> > formats that the old code made work are also in the list, we don't have
> > to touch bits_per_coded_sample for video (only audio).
> >
> > I translated the entries in pix_fmt_bps_avi in rawdec.c to entries in
> > ff_mxf_pixel_layouts (I left out the 4-bit case for now). This means we
> > don't have to set bits_per_coded_sample for AVMEDIA_TYPE_VIDEO in
> > mxf_parse_structural_metadata().
> >
> > Considering that the only sample we know of that should have a correct
> > PixelLayout is RGB24 this patch should work at least as well as the old
> > one. The only difference is rawdec translates 16 bps to PIX_FMT_RGB555,
> > while the similar entry here translates it to PIX_FMT_RGB565BE.
> >
> > Also, this patch passes the regtests.
> >
> > [...]
> >
> >   static int mxf_read_generic_descriptor(MXFDescriptor *descriptor, ByteIOContext *pb, int tag, int size, UID uid)
> > @@ -801,7 +800,8 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
> >                   st->codec->codec_id = container_ul->id;
> >               st->codec->width = descriptor->width;
> >               st->codec->height = descriptor->height;
> > -            st->codec->bits_per_coded_sample = descriptor->bits_per_sample; /* Uncompressed */
> > +            if (descriptor->got_pix_fmt)
> > +                st->codec->pix_fmt = descriptor->pix_fmt;
> 
> if codec_id == CODEC_ID_RAWVIDEO and drop got_pix_fmt, that should be 
> enough for now.
> 
> If it's needed later, we will update. Patch ok otherwise.

Ah, good idea. New patch attached. I also added a note about not using
the descriptors for encoding yet, until we get samples or official word
(the guy I talked to still hasn't heard anything).

Btw, the ML software messed up somehow. This message originally only
went to Baptiste, so I'm resending it to the list. Hopefully it'll work
this time

/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mxf_pix_fmt4.patch
Type: text/x-patch
Size: 5410 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100628/8db271ba/attachment.bin>
-------------- 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/20100628/8db271ba/attachment.pgp>