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

John Stebbins stebbins at jetheaddev.com
Wed Oct 4 20:58:19 EEST 2017


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?

-- 
John      GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01  83F0 49F1 D7B2 60D4 D0F7


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171004/33761bd5/attachment.sig>


More information about the ffmpeg-devel mailing list