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

[Tomas Härdin]

On Wed, 2010-06-30 at 00:02 -0700, Baptiste Coudurier wrote:
> On 6/28/10 12:46 AM, Tomas H?rdin wrote:
> > 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).
> >
> 
> Well, patch is ok if everything work, apply it.
> Thanks for the work, this is nice.
> 

Reran the tests just to be sure. Applied.

Last message went to your mail again.. Is gmail or Evolution doing something wrong?

/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/20100630/2f97d621/attachment.pgp>