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

[Michael Niedermayer]

On Wed, Sep 26, 2012 at 07:21:18PM +0200, Reimar Döffinger wrote:
> 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).

I do not know if this change is correct or what side effects this may
have. It looks odd to me that this was not set before this patch.
I would certainly favor this being seperate to make bisecting issues
it might cause easier

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120926/134d4eeb/attachment.asc>