[FFmpeg-devel] [PATCH 1/2] mov: fix decode of fragments that overlap in time

[Michael Niedermayer]

On Wed, Oct 04, 2017 at 10:58:19AM -0700, John Stebbins wrote:
> On 10/04/2017 10:13 AM, Michael Niedermayer wrote:
> > On Wed, Oct 04, 2017 at 08:18:59AM -0700, John Stebbins wrote:
> >> On 10/04/2017 03:50 AM, Michael Niedermayer wrote:
> >>> On Fri, Sep 29, 2017 at 08:54:08AM -0700, John Stebbins wrote:
> >>>> When keyframe intervals of dash segments are not perfectly aligned,
> >>>> fragments in the stream can overlap in time. Append new "trun" index
> >>>> entries to the end of the index instead of sorting by timestamp.
> >>>> Sorting by timestamp causes packets to be read out of decode order and
> >>>> results in decode errors.
> >>>> ---
> >>>>  libavformat/mov.c | 4 ++--
> >>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/libavformat/mov.c b/libavformat/mov.c
> >>>> index 899690d920..c7422cd9ed 100644
> >>>> --- a/libavformat/mov.c
> >>>> +++ b/libavformat/mov.c
> >>>> @@ -4340,8 +4340,8 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>>>                                    MOV_FRAG_SAMPLE_FLAG_DEPENDS_YES));
> >>>>          if (keyframe)
> >>>>              distance = 0;
> >>>> -        ctts_index = av_add_index_entry(st, offset, dts, sample_size, distance,
> >>>> -                                        keyframe ? AVINDEX_KEYFRAME : 0);
> >>>> +        ctts_index = add_index_entry(st, offset, dts, sample_size, distance,
> >>>> +                                     keyframe ? AVINDEX_KEYFRAME : 0);
> >>> can this lead to timestamps being out of order not just changing
> >>> from strictly monotone to monotone ?
> >>>
> >>> Maybe iam missing somehing but out of order could/would cause problems
> >>> with av_index_search_timestamp() and possibly others
> >>>
> >>>
> >> I'm not sure I understand the question.  But I think I can answer.  The new fragment can start before the last fragment
> >> ends. I'll make a concrete example.  Lets say the new fragment's first DTS is 10 frames before the end of the previous
> >> fragment. So the first DTS of the new fragment is before the timestamp of 10 entries in the index from the previous
> >> fragment.  av_add_index_entry searches the existing index and inserts the first sample of the new fragment in position
> >> nb_index_entries - 10 (and shifts the existing entries).  The next 9 samples of the new fragment get intermixed with the
> >> remaining 9 samples of the previous fragment, sorted by DTS. When the samples are read out, you get samples from the
> >> last fragment and the new fragment interleaved together causing decoding errors.
> >>
> >> Using add_index_entry will result in the timestamps in the index going backwards by 10 frames at the fragment boundary
> >> in this example.  In the other patch that accompanied this one, I've marked the samples from the new fragment that
> >> overlap previous samples with AVINDEX_DISCARD. ff_index_search_timestamp appears to be AVINDEX_DISCARD aware.  So I
> >> think av_index_search_timestamp will do the right thing.
> > yes, that makes sense now.
> > Please correct me if iam wrong but then patch 1 would introduce a
> > issue that the 2nd fixes. So both patches should be merged to avoid
> > this
> >
> > But theres another problem, trun can be read out of order, when one
> > seeks around, so the next might have to be put elsewhere than after the
> > previous
> >
> > thanks
> >
> 
> Hmm, can you describe the circumstances where this would happen.  I looked at the seek code and can't see any way for it
> to seek to the middle somewhere without first reading previous trun.  It looks to me like if avformat_seek_file or
> av_seek_frame fails to find the desired timestamp in the index it falls back to seek_frame_generic which seeks to the
> position of the last sample in the index and performs av_read_frame until it gets to the timestamp it wants.  Is there a
> path I've missed where it can skip to the middle of the file somehow?

I used
-rw-r----- 1 michael michael 66908195 Dec 11  2015 buck480p30_na.mp4
./ffplay buck480p30_na.mp4

(i can upload this if needed, i dont know where its from exactly)

and when seeking around by using the right mouse buttonq it sometimes read
trun chunks with lower times than previous (seen from the av_logs in
there)

I hope i made no mistake and would assume this happens with any file
with these chunks

...
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x7f3884000940] AVIndex stream 0, sample 151, offset 60134, dts 450000, size 194, distance 25, keyframe 0
...
Seek to 68% ( 0:07:11) of total duration ( 0:10:34)
...
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x7f3884000940] AVIndex stream 0, sample 152, offset 2b74fd6, dts 38757000, size 8284, distance 0, keyframe 1
...
Seek to 14% ( 0:01:29) of total duration ( 0:10:34)
...
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x7f3884000940] AVIndex stream 0, sample 152, offset 959164, dts 7749000, size 55027, distance 0, keyframe 1

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171005/ac5b479e/attachment.sig>