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

[Michael Niedermayer]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk

-------------- 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/20171004/5da426b5/attachment.sig>