[FFmpeg-devel] [PATCH] lavf/mov: ignore edit list with duration equals to 0 for covers art

[Yusuke Nakamura]

2017-01-19 18:33 GMT+09:00 Matthieu Bouron <matthieu.bouron at gmail.com>:

> On Thu, Jan 19, 2017 at 04:55:22AM +0900, Yusuke Nakamura wrote:
> > 2017-01-18 19:38 GMT+09:00 Matthieu Bouron <matthieu.bouron at gmail.com>:
> >
> > > Discards edit list with duration equals to 0 for video streams with
> only
> > > one frame and avoid discarding covers art muxed as a single frame video
> > > stream.
> > > ---
> > > Hello,
> > >
> > > The following patch discards single edit list with duration equals to 0
> > > for video streams with only one frame which prevents discarding covers
> art
> > > muxed as a single frame video stream.
> > >
> > > The patch can be extended to make it always discards single edit list
> with
> > > duration equals to 0 (even if the video streams has more than one
> sample).
> > > What do you think ?
> > >
> > > Matthieu
> > > ---
> > >  libavformat/mov.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > > index d1b929174d..88ffd0e5f2 100644
> > > --- a/libavformat/mov.c
> > > +++ b/libavformat/mov.c
> > > @@ -2988,6 +2988,17 @@ static void mov_fix_index(MOVContext *mov,
> AVStream
> > > *st)
> > >      if (!msc->elst_data || msc->elst_count <= 0 || nb_old <= 0) {
> > >          return;
> > >      }
> > > +
> > > +    // Discard edit list with duration equals to 0 for video streams
> with
> > > only
> > > +    // one frame and avoid discarding covers art muxed as a single
> frame
> > > video
> > > +    // stream
> > > +    if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> > > +        msc->chunk_count == 1 &&
> > > +        msc->elst_count == 1 &&
> > > +        msc->elst_data[0].duration == 0) {
> > >
> >
> > This can be false positive since segment_duration=0 could be used for
> > implicit segment_duration when movie fragments. The spec explicitly says
> > that only when initial movie has no samples but
> > w16161-14496-12-DefectReport-R4.docx implies that the 14496-12 spec will
> > also apply it to more generic cases. And why msc->chunk_count == 1?  The
> > chunk_count is the number of chunks but not samples.
> >
>
> If I understand correctly segment_duration=0 means implicit duration for
> all cases (fragmented / non-fragmented) ?  Does that mean we should
> discard the duration but honor the rest of the elst fields ?
>

Dunno if it also applies to non-fragmented movie but an example of the
current spec does not specify it's fragmented or not. Also I noticed the
latest File-format Meeting Report, which is found at Mp4-sys ML and a MPEG
meeting is held in this week, and it says

"Proposal #3, edit list: The question is whether a zero-length edit in a
non-empty movie means anything. It probably means an insert of zero
duration, which means it needs adjusting if de-fragmenting. This needs
saying in the spec.; update the defect report."

This time I can't insist which is correct or not. :<



> Regarding the use of msc->chunk_count I wrongly assumed that one chunk ==
> one video frame. I used originally st->nb_index_entries but I'm not sure
> if it's right though.
>
> Thanks,
> Matthieu
>
> [...]
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>