[FFmpeg-devel] [PATCH]Support more AVC-Intra files

[Reimar Döffinger]

On Wed, Sep 26, 2012 at 02:28:35PM +0200, Michael Niedermayer wrote:
> On Wed, Sep 26, 2012 at 12:13:53PM +0200, Carl Eugen Hoyos wrote:
> > Hi!
> > 
> > Followup to http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/151321
> > Attached patch by Reimar and Thomas Mundt fixes some AVC-Intra files from 
> > different tickets.
> 
> > It does not fix http://samples.ffmpeg.org/ffmpeg-
> > bugs/trac/ticket524/AVCI50.mov

That is probably due to progressive vs. interlaced detection issue,
probably caused by the part that makes mov.c set field_order being
removed.

> which file does it fix ?

I think it should fix all from tickets #1294, #524, #1666,
at least if the above is fixed.

> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index 16b8c12..1aff75b 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> 
> mixing mxfdec and movdec changes in the same patch is bad

I disagree, they implement basically exactly the same feature in both,
so it makes sense to do them together.

> > @@ -1484,10 +1484,6 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
> >          /* TODO: drop PictureEssenceCoding and SoundEssenceCompression, only check EssenceContainer */
> >          codec_ul = mxf_get_codec_ul(ff_mxf_codec_uls, &descriptor->essence_codec_ul);
> >          st->codec->codec_id = codec_ul->id;
> > -        if (descriptor->extradata) {
> > -            st->codec->extradata = descriptor->extradata;
> > -            st->codec->extradata_size = descriptor->extradata_size;
> > -        }
> >          if (st->codec->codec_type == AVMEDIA_TYPE_VIDEO) {
> >              source_track->intra_only = mxf_is_intra_only(descriptor);
> >              container_ul = mxf_get_codec_ul(mxf_picture_essence_container_uls, essence_container_ul);
> 
> > @@ -1501,6 +1497,7 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
> >                      av_log(mxf->fc, AV_LOG_INFO, "SegmentedFrame layout isn't currently supported\n");
> >                      break;
> >                  case FullFrame:
> > +                    st->codec->field_order = AV_FIELD_PROGRESSIVE;
> >                      break;
> >                  case OneField:
> >                      /* Every other line is stored and needs to be duplicated. */
> 
> this looks unrelated

No, without this field_order is not set, and without that the extradata
generation does not know whether to pick the extradata for
1080p or 1080i.
Or in other words: it does not work without that.
(This part could be applied independently before though, if
that is what you meant).