diff mbox

[FFmpeg-devel,mov] Fix trampling of ctts during seeks when sidx support is enabled.

Message ID CAPUDrwdjR+utG56pf6zUy4GcOS37RkVaVFA3h4bmMvgOfj_-9g@mail.gmail.com
State New
Headers show

Commit Message

Dale Curtis Aug. 10, 2017, 8:02 p.m. UTC 
On Tue, Aug 8, 2017 at 6:48 PM, Michael Niedermayer <michael@niedermayer.cc>
wrote:

>
> the fate test seems to fail:
>
> did i do something silly ?
>

Ah no, I did when I remuxed the test file. Updated expectations and test
clip at http://storage.googleapis.com/dalecurtis/buck480p30_na.mp4

- dale

Comments

Dale Curtis Aug. 18, 2017, 10:57 p.m. UTC | #1 
Anything else here? It'd be nice to get this landed soon if no one has any
other comments.

- dale

On Thu, Aug 10, 2017 at 1:02 PM, Dale Curtis <dalecurtis@chromium.org>
wrote:

> On Tue, Aug 8, 2017 at 6:48 PM, Michael Niedermayer <
> michael@niedermayer.cc> wrote:
>
>>
>> the fate test seems to fail:
>>
>> did i do something silly ?
>>
>
> Ah no, I did when I remuxed the test file. Updated expectations and test
> clip at http://storage.googleapis.com/dalecurtis/buck480p30_na.mp4
>
> - dale
>
Michael Niedermayer Aug. 19, 2017, 10:48 p.m. UTC | #2 
On Fri, Aug 18, 2017 at 03:57:45PM -0700, Dale Curtis wrote:
> Anything else here? It'd be nice to get this landed soon if no one has any
> other comments.

it appears to not apply cleanly anymore


> 
> - dale
> 
> On Thu, Aug 10, 2017 at 1:02 PM, Dale Curtis <dalecurtis@chromium.org>
> wrote:
> 
> > On Tue, Aug 8, 2017 at 6:48 PM, Michael Niedermayer <
> > michael@niedermayer.cc> wrote:
> >
> >>
> >> the fate test seems to fail:
> >>
> >> did i do something silly ?
> >>
> >
> > Ah no, I did when I remuxed the test file. Updated expectations and test
> > clip at http://storage.googleapis.com/dalecurtis/buck480p30_na.mp4
> >
> > - dale
> >
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Michael Niedermayer Aug. 19, 2017, 10:50 p.m. UTC | #3 
On Sun, Aug 20, 2017 at 12:48:30AM +0200, Michael Niedermayer wrote:
> On Fri, Aug 18, 2017 at 03:57:45PM -0700, Dale Curtis wrote:
> > Anything else here? It'd be nice to get this landed soon if no one has any
> > other comments.
> 
> it appears to not apply cleanly anymore

seems caused by f4544163b27615ecfff1b42d6acdb3672ac92399.

[...]
Dale Curtis Aug. 23, 2017, 10:39 p.m. UTC | #4 
On Sat, Aug 19, 2017 at 3:50 PM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Sun, Aug 20, 2017 at 12:48:30AM +0200, Michael Niedermayer wrote:
> > On Fri, Aug 18, 2017 at 03:57:45PM -0700, Dale Curtis wrote:
> > > Anything else here? It'd be nice to get this landed soon if no one has
> any
> > > other comments.
> >
> > it appears to not apply cleanly anymore
>
> seems caused by f4544163b27615ecfff1b42d6acdb3672ac92399.
>

Thanks rebased on ToT after chatting with Jacob to ensure my patch didn't
regress his issue.

- dale
Michael Niedermayer Aug. 24, 2017, 8:57 a.m. UTC | #5 
On Wed, Aug 23, 2017 at 03:39:01PM -0700, Dale Curtis wrote:
> On Sat, Aug 19, 2017 at 3:50 PM, Michael Niedermayer <michael@niedermayer.cc
> > wrote:
> 
> > On Sun, Aug 20, 2017 at 12:48:30AM +0200, Michael Niedermayer wrote:
> > > On Fri, Aug 18, 2017 at 03:57:45PM -0700, Dale Curtis wrote:
> > > > Anything else here? It'd be nice to get this landed soon if no one has
> > any
> > > > other comments.
> > >
> > > it appears to not apply cleanly anymore
> >
> > seems caused by f4544163b27615ecfff1b42d6acdb3672ac92399.
> >
> 
> Thanks rebased on ToT after chatting with Jacob to ensure my patch didn't
> regress his issue.
> 
> - dale

>  libavformat/isom.h       |    1 
>  libavformat/mov.c        |   92 +++++++++++++++++++-------------
>  tests/fate/seek.mak      |    3 +
>  tests/ref/seek/extra-mp4 |  134 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 195 insertions(+), 35 deletions(-)
> 98449b2f1a0464b474c3b17ee29850daa1b0081a  ctts_fix_v9.patch
> From b4b49b6b584b33e1da95a5d72b05fd9134ab28f9 Mon Sep 17 00:00:00 2001
> From: Dale Curtis <dalecurtis@chromium.org>
> Date: Mon, 17 Jul 2017 17:38:09 -0700
> Subject: [PATCH] Fix trampling of ctts during seeks when sidx support is
>  enabled.

applied

thanks

[...]
Michael Niedermayer Aug. 24, 2017, 9:27 a.m. UTC | #6 
On Wed, Aug 23, 2017 at 03:39:01PM -0700, Dale Curtis wrote:
> On Sat, Aug 19, 2017 at 3:50 PM, Michael Niedermayer <michael@niedermayer.cc
> > wrote:
> 
> > On Sun, Aug 20, 2017 at 12:48:30AM +0200, Michael Niedermayer wrote:
> > > On Fri, Aug 18, 2017 at 03:57:45PM -0700, Dale Curtis wrote:
> > > > Anything else here? It'd be nice to get this landed soon if no one has
> > any
> > > > other comments.
> > >
> > > it appears to not apply cleanly anymore
> >
> > seems caused by f4544163b27615ecfff1b42d6acdb3672ac92399.
> >
> 
> Thanks rebased on ToT after chatting with Jacob to ensure my patch didn't
> regress his issue.
> 
> - dale

>  libavformat/isom.h       |    1 
>  libavformat/mov.c        |   92 +++++++++++++++++++-------------
>  tests/fate/seek.mak      |    3 +
>  tests/ref/seek/extra-mp4 |  134 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 195 insertions(+), 35 deletions(-)
> 98449b2f1a0464b474c3b17ee29850daa1b0081a  ctts_fix_v9.patch
> From b4b49b6b584b33e1da95a5d72b05fd9134ab28f9 Mon Sep 17 00:00:00 2001
> From: Dale Curtis <dalecurtis@chromium.org>
> Date: Mon, 17 Jul 2017 17:38:09 -0700
> Subject: [PATCH] Fix trampling of ctts during seeks when sidx support is
>  enabled.
> 
> When sidx box support is enabled, the code will skip reading all
> trun boxes (each containing ctts entries for samples inthat box).
> 
> If seeks are attempted before all ctts values are known, the old
> code would dump ctts entries into the wrong location. These are
> then used to compute pts values which leads to out of order and
> incorrectly timestamped packets.
> 
> This patch fixes ctts processing by always using the index returned
> by av_add_index_entry() as the ctts_data index. When the index gains
> new entries old values are reshuffled as appropriate.
> 
> This approach makes sense since the mov demuxer is already relying
> on the mapping of AVIndex entries to samples for correct demuxing.
> 
> As a result of this all ctts entries are now 1-count. A followup
> change will be submitted to remove support for > 1 count entries
> which will simplify seeking.
> 
> Notes for future improvement:
> Probably there are other boxes (stts, stsc, etc) that are impacted
> by this issue... this patch only attempts to fix ctts since it
> completely breaks packet timestamping.
> 

> This patch continues using an array for the ctts data, which is not
> the most ideal given the rearrangement that needs to happen (via
> memmove as new entries are read in). Ideally AVIndex and the ctts
> data would be set-type structures so addition is always worst case
> O(lg(n)) instead of the O(n^2) that exists now; this slowdown is
> noticeable during seeks.

can the insertions be done in groups instead of one at a time ?
so that it basically merges 2 lists (O(n)) instead of inserting
one at a time O(n^2)
?
This would significantly improve the worst case while not needing
to change the data structures
(of course iam not against changing the data structures if someone wants
to do the work)

[...]
Dale Curtis Aug. 25, 2017, 12:06 a.m. UTC | #7 
On Thu, Aug 24, 2017 at 2:27 AM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

>
> can the insertions be done in groups instead of one at a time ?
> so that it basically merges 2 lists (O(n)) instead of inserting
> one at a time O(n^2)
> ?
> This would significantly improve the worst case while not needing
> to change the data structures
> (of course iam not against changing the data structures if someone wants
> to do the work)
>

Unfortunately this is hard / impossible to do if I understand what you're
asking for correctly. Here's my response to the same suggestion from Rodger
above: "We could speculatively move all entries based on the first insert
and total entries count, but their are several conditionals in
av_add_index_entry() which may cause a bail out and such failure would be
unrecoverable (maybe painfully?) if we moved everything ahead of time."

- dale
Michael Niedermayer Aug. 25, 2017, 5:18 p.m. UTC | #8 
On Thu, Aug 24, 2017 at 05:06:16PM -0700, Dale Curtis wrote:
> On Thu, Aug 24, 2017 at 2:27 AM, Michael Niedermayer <michael@niedermayer.cc
> > wrote:
> 
> >
> > can the insertions be done in groups instead of one at a time ?
> > so that it basically merges 2 lists (O(n)) instead of inserting
> > one at a time O(n^2)
> > ?
> > This would significantly improve the worst case while not needing
> > to change the data structures
> > (of course iam not against changing the data structures if someone wants
> > to do the work)
> >
> 
> Unfortunately this is hard / impossible to do if I understand what you're
> asking for correctly. Here's my response to the same suggestion from Rodger
> above: "We could speculatively move all entries based on the first insert
> and total entries count, but their are several conditionals in
> av_add_index_entry() which may cause a bail out and such failure would be
> unrecoverable (maybe painfully?) if we moved everything ahead of time."

hmm

maybe i misunderstand but assuming we insert a block of dummy blank
entries (without breaking monotonicity) and keep a pointer to that
block

then add entries with a av_add_index_entry()
and in case of failure remove the blank entries

this would result in the same as now and seems relativly simple
it would not need memmove and in general would not need a log n
search if each falls on the first spot of the block

or am i missing something ?





[...]
Daniel Glöckner Aug. 28, 2017, 12:01 a.m. UTC | #9 
On Thu, Aug 24, 2017 at 10:57:43AM +0200, Michael Niedermayer wrote:
> > From b4b49b6b584b33e1da95a5d72b05fd9134ab28f9 Mon Sep 17 00:00:00 2001
> > From: Dale Curtis <dalecurtis@chromium.org>
> > Date: Mon, 17 Jul 2017 17:38:09 -0700
> > Subject: [PATCH] Fix trampling of ctts during seeks when sidx support is
> >  enabled.
> 
> applied
> 
> thanks

With this commit I can no longer reproduce the problem described in
ticket 6560, but I now get lots of

[mov,mp4,m4a,3gp,3g2,mj2 @ 0x9942020] Failed to add index entry

messages. These messages vanish if I also apply the patch that I posted
three weeks ago ( https://patchwork.ffmpeg.org/patch/4632/ ) and that
has so far received no comments.

Best regards,

  Daniel
Dale Curtis Aug. 28, 2017, 11:07 p.m. UTC | #10 
On Fri, Aug 25, 2017 at 10:18 AM, Michael Niedermayer <
michael@niedermayer.cc> wrote:
>
> hmm
>
> maybe i misunderstand but assuming we insert a block of dummy blank
> entries (without breaking monotonicity) and keep a pointer to that
> block
>
> then add entries with a av_add_index_entry()
> and in case of failure remove the blank entries
>
> this would result in the same as now and seems relativly simple
> it would not need memmove and in general would not need a log n
> search if each falls on the first spot of the block
>
> or am i missing something ?
>

This patch does what you're suggesting, but I'm not confident it's right in
all cases.

- dale
Carl Eugen Hoyos Nov. 22, 2017, 10:36 p.m. UTC | #11 
2017-08-24 0:39 GMT+02:00 Dale Curtis <dalecurtis@chromium.org>:

> -        sc->ctts_data[ctts_count].count    = count;
> -        sc->ctts_data[ctts_count].duration = duration;
> -        ctts_count++;
> +        /* Expand entries such that we have a 1-1 mapping with samples. */
> +        for (j = 0; j < count; j++)
> +            add_ctts_entry(&sc->ctts_data, &ctts_count, &sc->ctts_allocated_size, 1, duration);

count is a 32bit value read from the file, so this hunk makes
the demuxer allocate huge amount of memories for some
files.

Is there an upper limit for count?

Carl Eugen
John Stebbins Nov. 23, 2017, 12:30 a.m. UTC | #12 
On 11/22/2017 02:36 PM, Carl Eugen Hoyos wrote:
> 2017-08-24 0:39 GMT+02:00 Dale Curtis <dalecurtis@chromium.org>:
>
>> -        sc->ctts_data[ctts_count].count    = count;
>> -        sc->ctts_data[ctts_count].duration = duration;
>> -        ctts_count++;
>> +        /* Expand entries such that we have a 1-1 mapping with samples. */
>> +        for (j = 0; j < count; j++)
>> +            add_ctts_entry(&sc->ctts_data, &ctts_count, &sc->ctts_allocated_size, 1, duration);
> count is a 32bit value read from the file, so this hunk makes
> the demuxer allocate huge amount of memories for some
> files.
>
> Is there an upper limit for count?
>
>

In practice, if a valid mp4 blows up due to this ctts allocation, it's also going to blow up when AVIndexEntries is
allocated for the samples.  An invalid mp4 can do anything of course.
Carl Eugen Hoyos Nov. 23, 2017, 1:26 a.m. UTC | #13 
2017-11-23 1:30 GMT+01:00 John Stebbins <stebbins@jetheaddev.com>:
> On 11/22/2017 02:36 PM, Carl Eugen Hoyos wrote:
>> 2017-08-24 0:39 GMT+02:00 Dale Curtis <dalecurtis@chromium.org>:
>>
>>> -        sc->ctts_data[ctts_count].count    = count;
>>> -        sc->ctts_data[ctts_count].duration = duration;
>>> -        ctts_count++;
>>> +        /* Expand entries such that we have a 1-1 mapping with samples. */
>>> +        for (j = 0; j < count; j++)
>>> +            add_ctts_entry(&sc->ctts_data, &ctts_count, &sc->ctts_allocated_size, 1, duration);
>> count is a 32bit value read from the file, so this hunk makes
>> the demuxer allocate huge amount of memories for some
>> files.
>>
>> Is there an upper limit for count?
>
> In practice, if a valid mp4 blows up due to this ctts allocation,
> it's also going to blow up when AVIndexEntries is allocated
> for the samples.

> An invalid mp4 can do anything of course.

This is about invalid files allocating >1GB.

Carl Eugen
John Stebbins Nov. 23, 2017, 6:45 p.m. UTC | #14 
On 11/22/2017 05:26 PM, Carl Eugen Hoyos wrote:
> 2017-11-23 1:30 GMT+01:00 John Stebbins <stebbins@jetheaddev.com>:
>> On 11/22/2017 02:36 PM, Carl Eugen Hoyos wrote:
>>> 2017-08-24 0:39 GMT+02:00 Dale Curtis <dalecurtis@chromium.org>:
>>>
>>>> -        sc->ctts_data[ctts_count].count    = count;
>>>> -        sc->ctts_data[ctts_count].duration = duration;
>>>> -        ctts_count++;
>>>> +        /* Expand entries such that we have a 1-1 mapping with samples. */
>>>> +        for (j = 0; j < count; j++)
>>>> +            add_ctts_entry(&sc->ctts_data, &ctts_count, &sc->ctts_allocated_size, 1, duration);
>>> count is a 32bit value read from the file, so this hunk makes
>>> the demuxer allocate huge amount of memories for some
>>> files.
>>>
>>> Is there an upper limit for count?
>> In practice, if a valid mp4 blows up due to this ctts allocation,
>> it's also going to blow up when AVIndexEntries is allocated
>> for the samples.
>> An invalid mp4 can do anything of course.
> This is about invalid files allocating >1GB.
>
>

Ah, ok.  The practical limit would be the number of samples (sc->sample_count).  But you can't be certain this is set
before the ctts box is parsed (the value is determined when parsing stsz box).  You can be certain it is set before
mov_build_index is called.  So perhaps revert this part and then add code to mov_build_index to expand the ctts_data
entries there.  This would solve the invalid mp4 alloc issues while still preserving the fix for trampling of ctts.
Carl Eugen Hoyos Nov. 23, 2017, 11:54 p.m. UTC | #15 
2017-11-23 19:45 GMT+01:00 John Stebbins <stebbins@jetheaddev.com>:
> On 11/22/2017 05:26 PM, Carl Eugen Hoyos wrote:
>> 2017-11-23 1:30 GMT+01:00 John Stebbins <stebbins@jetheaddev.com>:
>>> On 11/22/2017 02:36 PM, Carl Eugen Hoyos wrote:
>>>> 2017-08-24 0:39 GMT+02:00 Dale Curtis <dalecurtis@chromium.org>:
>>>>
>>>>> -        sc->ctts_data[ctts_count].count    = count;
>>>>> -        sc->ctts_data[ctts_count].duration = duration;
>>>>> -        ctts_count++;
>>>>> +        /* Expand entries such that we have a 1-1 mapping with samples. */
>>>>> +        for (j = 0; j < count; j++)
>>>>> +            add_ctts_entry(&sc->ctts_data, &ctts_count, &sc->ctts_allocated_size, 1, duration);
>>>> count is a 32bit value read from the file, so this hunk makes
>>>> the demuxer allocate huge amount of memories for some
>>>> files.
>>>>
>>>> Is there an upper limit for count?
>>> In practice, if a valid mp4 blows up due to this ctts allocation,
>>> it's also going to blow up when AVIndexEntries is allocated
>>> for the samples.
>>> An invalid mp4 can do anything of course.
>> This is about invalid files allocating >1GB.
>
> Ah, ok.  The practical limit would be the number of samples (sc->sample_count).
> But you can't be certain this is set before the ctts box is parsed (the value is
> determined when parsing stsz box).  You can be certain it is set before
> mov_build_index is called.  So perhaps revert this part and then add code to
> mov_build_index to expand the ctts_data entries there.  This would solve the
> invalid mp4 alloc issues while still preserving the fix for trampling of ctts.

@Dale:
Could you do that?

Thank you, Carl Eugen
Dale Curtis Nov. 27, 2017, 10:50 p.m. UTC | #16 
On Thu, Nov 23, 2017 at 3:54 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com>
wrote:
>
> @Dale:
> Could you do that?
>

Thanks to John for putting out a patch for this.

- dale
diff mbox

Patch

From 6e77ff3deaa6e0ac04fb4e51dba1d4a0e69e9d5d Mon Sep 17 00:00:00 2001
From: Dale Curtis <dalecurtis@chromium.org>
Date: Mon, 17 Jul 2017 17:38:09 -0700
Subject: [PATCH] Fix trampling of ctts during seeks when sidx support is
 enabled.

When sidx box support is enabled, the code will skip reading all
trun boxes (each containing ctts entries for samples inthat box).

If seeks are attempted before all ctts values are known, the old
code would dump ctts entries into the wrong location. These are
then used to compute pts values which leads to out of order and
incorrectly timestamped packets.

This patch fixes ctts processing by always using the index returned
by av_add_index_entry() as the ctts_data index. When the index gains
new entries old values are reshuffled as appropriate.

This approach makes sense since the mov demuxer is already relying
on the mapping of AVIndex entries to samples for correct demuxing.

As a result of this all ctts entries are now 1-count. A followup
change will be submitted to remove support for > 1 count entries
which will simplify seeking.

Notes for future improvement:
Probably there are other boxes (stts, stsc, etc) that are impacted
by this issue... this patch only attempts to fix ctts since it
completely breaks packet timestamping.

This patch continues using an array for the ctts data, which is not
the most ideal given the rearrangement that needs to happen (via
memmove as new entries are read in). Ideally AVIndex and the ctts
data would be set-type structures so addition is always worst case
O(lg(n)) instead of the O(n^2) that exists now; this slowdown is
noticeable during seeks.

Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
---
 libavformat/isom.h       |   1 +
 libavformat/mov.c        |  79 ++++++++++++++++++----------
 tests/fate/seek.mak      |   3 ++
 tests/ref/seek/extra-mp4 | 134 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 189 insertions(+), 28 deletions(-)
 create mode 100644 tests/ref/seek/extra-mp4

diff --git a/libavformat/isom.h b/libavformat/isom.h
index ff009b0896..fdd98c28f5 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -137,6 +137,7 @@  typedef struct MOVStreamContext {
     unsigned int stts_count;
     MOVStts *stts_data;
     unsigned int ctts_count;
+    unsigned int ctts_allocated_size;
     MOVStts *ctts_data;
     unsigned int stsc_count;
     MOVStsc *stsc_data;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 63f84be782..85377f39a9 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -74,6 +74,8 @@  typedef struct MOVParseTableEntry {
 
 static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom);
 static int mov_read_mfra(MOVContext *c, AVIOContext *f);
+static int64_t add_ctts_entry(MOVStts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size,
+                              int count, int duration);
 
 static int mov_metadata_track_or_disc_number(MOVContext *c, AVIOContext *pb,
                                              unsigned len, const char *key)
@@ -2708,7 +2710,7 @@  static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
     AVStream *st;
     MOVStreamContext *sc;
-    unsigned int i, entries, ctts_count = 0;
+    unsigned int i, j, entries, ctts_count = 0;
 
     if (c->fc->nb_streams < 1)
         return 0;
@@ -2726,7 +2728,7 @@  static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     if (entries >= UINT_MAX / sizeof(*sc->ctts_data))
         return AVERROR_INVALIDDATA;
     av_freep(&sc->ctts_data);
-    sc->ctts_data = av_realloc(NULL, entries * sizeof(*sc->ctts_data));
+    sc->ctts_data = av_fast_realloc(NULL, &sc->ctts_allocated_size, entries * sizeof(*sc->ctts_data));
     if (!sc->ctts_data)
         return AVERROR(ENOMEM);
 
@@ -2741,9 +2743,9 @@  static int mov_read_ctts(MOVContext *c, AVIOContext *pb, MOVAtom atom)
             continue;
         }
 
-        sc->ctts_data[ctts_count].count    = count;
-        sc->ctts_data[ctts_count].duration = duration;
-        ctts_count++;
+        /* Expand entries such that we have a 1-1 mapping with samples. */
+        for (j = 0; j < count; j++)
+            add_ctts_entry(&sc->ctts_data, &ctts_count, &sc->ctts_allocated_size, 1, duration);
 
         av_log(c->fc, AV_LOG_TRACE, "count=%d, duration=%d\n",
                 count, duration);
@@ -3046,7 +3048,6 @@  static void mov_fix_index(MOVContext *mov, AVStream *st)
     int64_t index;
     int64_t index_ctts_count;
     int flags;
-    unsigned int ctts_allocated_size = 0;
     int64_t start_dts = 0;
     int64_t edit_list_media_time_dts = 0;
     int64_t edit_list_start_encountered = 0;
@@ -3081,6 +3082,7 @@  static void mov_fix_index(MOVContext *mov, AVStream *st)
     msc->ctts_count = 0;
     msc->ctts_index = 0;
     msc->ctts_sample = 0;
+    msc->ctts_allocated_size = 0;
 
     // If the dts_shift is positive (in case of negative ctts values in mov),
     // then negate the DTS by dts_shift
@@ -3190,7 +3192,7 @@  static void mov_fix_index(MOVContext *mov, AVStream *st)
                 ctts_sample_old++;
                 if (ctts_sample_old == ctts_data_old[ctts_index_old].count) {
                     if (add_ctts_entry(&msc->ctts_data, &msc->ctts_count,
-                                       &ctts_allocated_size,
+                                       &msc->ctts_allocated_size,
                                        ctts_data_old[ctts_index_old].count - edit_list_start_ctts_sample,
                                        ctts_data_old[ctts_index_old].duration) == -1) {
                         av_log(mov->fc, AV_LOG_ERROR, "Cannot add CTTS entry %"PRId64" - {%"PRId64", %d}\n",
@@ -3289,7 +3291,7 @@  static void mov_fix_index(MOVContext *mov, AVStream *st)
 
                 if (ctts_data_old && ctts_sample_old != 0) {
                     if (add_ctts_entry(&msc->ctts_data, &msc->ctts_count,
-                                       &ctts_allocated_size,
+                                       &msc->ctts_allocated_size,
                                        ctts_sample_old - edit_list_start_ctts_sample,
                                        ctts_data_old[ctts_index_old].duration) == -1) {
                         av_log(mov->fc, AV_LOG_ERROR, "Cannot add CTTS entry %"PRId64" - {%"PRId64", %d}\n",
@@ -4259,7 +4261,7 @@  static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     int64_t dts;
     int data_offset = 0;
     unsigned entries, first_sample_flags = frag->flags;
-    int flags, distance, i, err;
+    int flags, distance, i;
 
     for (i = 0; i < c->fc->nb_streams; i++) {
         if (c->fc->streams[i]->id == frag->track_id) {
@@ -4287,21 +4289,20 @@  static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     if (!sc->ctts_count && sc->sample_count)
     {
         /* Complement ctts table if moov atom doesn't have ctts atom. */
-        ctts_data = av_realloc(NULL, sizeof(*sc->ctts_data));
+        ctts_data = av_fast_realloc(NULL, &sc->ctts_allocated_size, sizeof(*sc->ctts_data) * sc->sample_count);
         if (!ctts_data)
             return AVERROR(ENOMEM);
+        /* Don't use a count greater than 1 here since it will leave a gap in
+         * the ctts index which the code below relies on being sequential. */
         sc->ctts_data = ctts_data;
-        sc->ctts_data[sc->ctts_count].count = sc->sample_count;
-        sc->ctts_data[sc->ctts_count].duration = 0;
-        sc->ctts_count++;
+        for (i = 0; i < sc->sample_count; i++) {
+            sc->ctts_data[sc->ctts_count].count = 1;
+            sc->ctts_data[sc->ctts_count].duration = 0;
+            sc->ctts_count++;
+        }
     }
     if ((uint64_t)entries+sc->ctts_count >= UINT_MAX/sizeof(*sc->ctts_data))
         return AVERROR_INVALIDDATA;
-    if ((err = av_reallocp_array(&sc->ctts_data, entries + sc->ctts_count,
-                                 sizeof(*sc->ctts_data))) < 0) {
-        sc->ctts_count = 0;
-        return err;
-    }
     if (flags & MOV_TRUN_DATA_OFFSET)        data_offset        = avio_rb32(pb);
     if (flags & MOV_TRUN_FIRST_SAMPLE_FLAGS) first_sample_flags = avio_rb32(pb);
     dts    = sc->track_end - sc->time_offset;
@@ -4312,26 +4313,28 @@  static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         unsigned sample_size = frag->size;
         int sample_flags = i ? frag->flags : first_sample_flags;
         unsigned sample_duration = frag->duration;
+        unsigned ctts_duration = 0;
         int keyframe = 0;
+        int ctts_index = 0;
+        int old_nb_index_entries = st->nb_index_entries;
 
         if (flags & MOV_TRUN_SAMPLE_DURATION) sample_duration = avio_rb32(pb);
         if (flags & MOV_TRUN_SAMPLE_SIZE)     sample_size     = avio_rb32(pb);
         if (flags & MOV_TRUN_SAMPLE_FLAGS)    sample_flags    = avio_rb32(pb);
-        sc->ctts_data[sc->ctts_count].count = 1;
-        sc->ctts_data[sc->ctts_count].duration = (flags & MOV_TRUN_SAMPLE_CTS) ?
-                                                  avio_rb32(pb) : 0;
-        mov_update_dts_shift(sc, sc->ctts_data[sc->ctts_count].duration);
+        if (flags & MOV_TRUN_SAMPLE_CTS)      ctts_duration   = avio_rb32(pb);
+
+        mov_update_dts_shift(sc, ctts_duration);
         if (frag->time != AV_NOPTS_VALUE) {
             if (c->use_mfra_for == FF_MOV_FLAG_MFRA_PTS) {
                 int64_t pts = frag->time;
                 av_log(c->fc, AV_LOG_DEBUG, "found frag time %"PRId64
                         " sc->dts_shift %d ctts.duration %d"
                         " sc->time_offset %"PRId64" flags & MOV_TRUN_SAMPLE_CTS %d\n", pts,
-                        sc->dts_shift, sc->ctts_data[sc->ctts_count].duration,
+                        sc->dts_shift, ctts_duration,
                         sc->time_offset, flags & MOV_TRUN_SAMPLE_CTS);
                 dts = pts - sc->dts_shift;
                 if (flags & MOV_TRUN_SAMPLE_CTS) {
-                    dts -= sc->ctts_data[sc->ctts_count].duration;
+                    dts -= ctts_duration;
                 } else {
                     dts -= sc->time_offset;
                 }
@@ -4343,7 +4346,7 @@  static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
             }
             frag->time = AV_NOPTS_VALUE;
         }
-        sc->ctts_count++;
+
         if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO)
             keyframe = 1;
         else
@@ -4352,11 +4355,31 @@  static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
                                   MOV_FRAG_SAMPLE_FLAG_DEPENDS_YES));
         if (keyframe)
             distance = 0;
-        err = av_add_index_entry(st, offset, dts, sample_size, distance,
-                                 keyframe ? AVINDEX_KEYFRAME : 0);
-        if (err < 0) {
+        ctts_index = av_add_index_entry(st, offset, dts, sample_size, distance,
+                                        keyframe ? AVINDEX_KEYFRAME : 0);
+        if (ctts_index >= 0 && old_nb_index_entries < st->nb_index_entries) {
+            unsigned int size_needed = st->nb_index_entries * sizeof(*sc->ctts_data);
+            unsigned int request_size = size_needed > sc->ctts_allocated_size ?
+                FFMAX(size_needed, 2 * sc->ctts_allocated_size) : size_needed;
+            ctts_data = av_fast_realloc(sc->ctts_data, &sc->ctts_allocated_size, request_size);
+            if (!ctts_data) {
+                av_freep(&sc->ctts_data);
+                return AVERROR(ENOMEM);
+            }
+
+            sc->ctts_data = ctts_data;
+            if (ctts_index != old_nb_index_entries) {
+                memmove(sc->ctts_data + ctts_index + 1, sc->ctts_data + ctts_index,
+                        sizeof(*sc->ctts_data) * (sc->ctts_count - ctts_index));
+            }
+
+            sc->ctts_data[ctts_index].count = 1;
+            sc->ctts_data[ctts_index].duration = ctts_duration;
+            sc->ctts_count++;
+        } else {
             av_log(c->fc, AV_LOG_ERROR, "Failed to add index entry\n");
         }
+
         av_log(c->fc, AV_LOG_TRACE, "AVIndex stream %d, sample %u, offset %"PRIx64", dts %"PRId64", "
                 "size %u, distance %d, keyframe %d\n", st->index, sc->sample_count+i,
                 offset, dts, sample_size, distance, keyframe);
diff --git a/tests/fate/seek.mak b/tests/fate/seek.mak
index f835da5226..1a6e584987 100644
--- a/tests/fate/seek.mak
+++ b/tests/fate/seek.mak
@@ -248,7 +248,10 @@  FATE_SEEK += $(FATE_SEEK_LAVF-yes:%=fate-seek-lavf-%)
 FATE_SEEK_EXTRA-$(CONFIG_MP3_DEMUXER)   += fate-seek-extra-mp3
 FATE_SEEK_EXTRA-$(call ALLYES, CACHE_PROTOCOL PIPE_PROTOCOL MP3_DEMUXER) += fate-seek-cache-pipe
 FATE_SEEK_EXTRA-$(CONFIG_MATROSKA_DEMUXER) += fate-seek-mkv-codec-delay
+FATE_SEEK_EXTRA-$(CONFIG_MOV_DEMUXER) += fate-seek-extra-mp4
+
 fate-seek-extra-mp3:  CMD = run libavformat/tests/seek$(EXESUF) $(TARGET_SAMPLES)/gapless/gapless.mp3 -fastseek 1
+fate-seek-extra-mp4:  CMD = run libavformat/tests/seek$(EXESUF) $(TARGET_SAMPLES)/mov/buck480p30_na.mp4 -duration 180 -frames 4
 fate-seek-cache-pipe: CMD = cat $(TARGET_SAMPLES)/gapless/gapless.mp3 | run libavformat/tests/seek$(EXESUF) cache:pipe:0 -read_ahead_limit -1
 fate-seek-mkv-codec-delay:   CMD = run libavformat/tests/seek$(EXESUF) $(TARGET_SAMPLES)/mkv/codec_delay_opus.mkv
 
diff --git a/tests/ref/seek/extra-mp4 b/tests/ref/seek/extra-mp4
new file mode 100644
index 0000000000..c25544c095
--- /dev/null
+++ b/tests/ref/seek/extra-mp4
@@ -0,0 +1,134 @@ 
+ret: 0         st: 0 flags:1 dts:-0.033333 pts: 0.000000 pos:   1287 size:   183
+ret: 0         st: 0 flags:0 dts: 0.000000 pts: 0.033333 pos:   1470 size:    24
+ret: 0         st: 0 flags:0 dts: 0.033333 pts: 0.066667 pos:   1494 size:  6779
+ret: 0         st: 0 flags:0 dts: 0.066667 pts: 0.100000 pos:   8273 size: 11041
+ret: 0         st:-1 flags:0  ts:-1.000000
+ret: 0         st: 0 flags:1 dts:-0.033333 pts: 0.000000 pos:   1287 size:   183
+ret: 0         st: 0 flags:0 dts: 0.000000 pts: 0.033333 pos:   1470 size:    24
+ret: 0         st: 0 flags:0 dts: 0.033333 pts: 0.066667 pos:   1494 size:  6779
+ret: 0         st: 0 flags:0 dts: 0.066667 pts: 0.100000 pos:   8273 size: 11041
+ret: 0         st:-1 flags:1  ts: 101.894167
+ret: 0         st: 0 flags:1 dts: 101.300000 pts: 101.333333 pos:10982311 size: 67237
+ret: 0         st: 0 flags:0 dts: 101.333333 pts: 101.433333 pos:11049548 size:  3524
+ret: 0         st: 0 flags:0 dts: 101.366667 pts: 101.366667 pos:11053072 size:   562
+ret: 0         st: 0 flags:0 dts: 101.400000 pts: 101.400000 pos:11053634 size:   599
+ret: 0         st: 0 flags:0  ts: 24.788333
+ret: 0         st: 0 flags:1 dts: 25.300000 pts: 25.333333 pos:2607246 size: 40273
+ret: 0         st: 0 flags:0 dts: 25.333333 pts: 25.433333 pos:2647519 size:  2959
+ret: 0         st: 0 flags:0 dts: 25.366667 pts: 25.366667 pos:2650478 size:   197
+ret: 0         st: 0 flags:0 dts: 25.400000 pts: 25.400000 pos:2650675 size:   230
+ret: 0         st: 0 flags:1  ts: 127.682500
+ret: 0         st: 0 flags:1 dts: 126.633333 pts: 126.666667 pos:13359975 size: 68741
+ret: 0         st: 0 flags:0 dts: 126.666667 pts: 126.766667 pos:13428716 size:  2914
+ret: 0         st: 0 flags:0 dts: 126.700000 pts: 126.700000 pos:13431630 size:   781
+ret: 0         st: 0 flags:0 dts: 126.733333 pts: 126.733333 pos:13432411 size:   817
+ret: 0         st:-1 flags:0  ts: 50.576668
+ret: 0         st: 0 flags:1 dts: 50.600000 pts: 50.633333 pos:5858254 size: 67903
+ret: 0         st: 0 flags:0 dts: 50.633333 pts: 50.733333 pos:5926157 size:  1307
+ret: 0         st: 0 flags:0 dts: 50.666667 pts: 50.666667 pos:5927464 size:   150
+ret: 0         st: 0 flags:0 dts: 50.700000 pts: 50.700000 pos:5927614 size:   176
+ret: 0         st:-1 flags:1  ts: 153.470835
+ret: 0         st: 0 flags:1 dts: 153.466667 pts: 153.500000 pos:15867700 size: 96169
+ret: 0         st: 0 flags:0 dts: 153.500000 pts: 153.533333 pos:15963869 size:   785
+ret: 0         st: 0 flags:0 dts: 153.533333 pts: 153.633333 pos:15964654 size:  3135
+ret: 0         st: 0 flags:0 dts: 153.566667 pts: 153.566667 pos:15967789 size:   859
+ret: 0         st: 0 flags:0  ts: 76.365000
+ret: 0         st: 0 flags:1 dts: 77.833333 pts: 77.866667 pos:8659657 size: 41182
+ret: 0         st: 0 flags:0 dts: 77.866667 pts: 77.966667 pos:8700839 size:  4197
+ret: 0         st: 0 flags:0 dts: 77.900000 pts: 77.900000 pos:8705036 size:   653
+ret: 0         st: 0 flags:0 dts: 77.933333 pts: 77.933333 pos:8705689 size:   751
+ret: 0         st: 0 flags:1  ts:-0.740833
+ret: 0         st: 0 flags:1 dts:-0.033333 pts: 0.000000 pos:   1287 size:   183
+ret: 0         st: 0 flags:0 dts: 0.000000 pts: 0.033333 pos:   1470 size:    24
+ret: 0         st: 0 flags:0 dts: 0.033333 pts: 0.066667 pos:   1494 size:  6779
+ret: 0         st: 0 flags:0 dts: 0.066667 pts: 0.100000 pos:   8273 size: 11041
+ret: 0         st:-1 flags:0  ts: 102.153336
+ret: 0         st: 0 flags:1 dts: 104.066667 pts: 104.100000 pos:11116461 size:112929
+ret: 0         st: 0 flags:0 dts: 104.100000 pts: 104.133333 pos:11229390 size:   585
+ret: 0         st: 0 flags:0 dts: 104.133333 pts: 104.166667 pos:11229975 size:   797
+ret: 0         st: 0 flags:0 dts: 104.166667 pts: 104.200000 pos:11230772 size:   810
+ret: 0         st:-1 flags:1  ts: 25.047503
+ret: 0         st: 0 flags:1 dts: 20.233333 pts: 20.266667 pos:2223959 size: 51823
+ret: 0         st: 0 flags:0 dts: 20.266667 pts: 20.300000 pos:2275782 size:   488
+ret: 0         st: 0 flags:0 dts: 20.300000 pts: 20.400000 pos:2276270 size:   670
+ret: 0         st: 0 flags:0 dts: 20.333333 pts: 20.333333 pos:2276940 size:    84
+ret: 0         st: 0 flags:0  ts: 127.941667
+ret: 0         st: 0 flags:1 dts: 131.233333 pts: 131.266667 pos:13727953 size: 62229
+ret: 0         st: 0 flags:0 dts: 131.266667 pts: 131.366667 pos:13790182 size:  2349
+ret: 0         st: 0 flags:0 dts: 131.300000 pts: 131.300000 pos:13792531 size:   571
+ret: 0         st: 0 flags:0 dts: 131.333333 pts: 131.333333 pos:13793102 size:  1190
+ret: 0         st: 0 flags:1  ts: 50.835833
+ret: 0         st: 0 flags:1 dts: 50.600000 pts: 50.633333 pos:5858254 size: 67903
+ret: 0         st: 0 flags:0 dts: 50.633333 pts: 50.733333 pos:5926157 size:  1307
+ret: 0         st: 0 flags:0 dts: 50.666667 pts: 50.666667 pos:5927464 size:   150
+ret: 0         st: 0 flags:0 dts: 50.700000 pts: 50.700000 pos:5927614 size:   176
+ret: 0         st:-1 flags:0  ts: 153.730004
+ret: 0         st: 0 flags:1 dts: 157.033333 pts: 157.066667 pos:16225365 size: 82738
+ret: 0         st: 0 flags:0 dts: 157.066667 pts: 157.166667 pos:16308103 size:  2273
+ret: 0         st: 0 flags:0 dts: 157.100000 pts: 157.100000 pos:16310376 size:   350
+ret: 0         st: 0 flags:0 dts: 157.133333 pts: 157.133333 pos:16310726 size:   337
+ret: 0         st:-1 flags:1  ts: 76.624171
+ret: 0         st: 0 flags:1 dts: 75.966667 pts: 76.000000 pos:8520178 size: 94395
+ret: 0         st: 0 flags:0 dts: 76.000000 pts: 76.100000 pos:8614573 size:   483
+ret: 0         st: 0 flags:0 dts: 76.033333 pts: 76.033333 pos:8615056 size:    37
+ret: 0         st: 0 flags:0 dts: 76.066667 pts: 76.066667 pos:8615093 size:    56
+ret: 0         st: 0 flags:0  ts:-0.481667
+ret: 0         st: 0 flags:1 dts:-0.033333 pts: 0.000000 pos:   1287 size:   183
+ret: 0         st: 0 flags:0 dts: 0.000000 pts: 0.033333 pos:   1470 size:    24
+ret: 0         st: 0 flags:0 dts: 0.033333 pts: 0.066667 pos:   1494 size:  6779
+ret: 0         st: 0 flags:0 dts: 0.066667 pts: 0.100000 pos:   8273 size: 11041
+ret: 0         st: 0 flags:1  ts: 102.412500
+ret: 0         st: 0 flags:1 dts: 101.300000 pts: 101.333333 pos:10982311 size: 67237
+ret: 0         st: 0 flags:0 dts: 101.333333 pts: 101.433333 pos:11049548 size:  3524
+ret: 0         st: 0 flags:0 dts: 101.366667 pts: 101.366667 pos:11053072 size:   562
+ret: 0         st: 0 flags:0 dts: 101.400000 pts: 101.400000 pos:11053634 size:   599
+ret: 0         st:-1 flags:0  ts: 25.306672
+ret: 0         st: 0 flags:1 dts: 27.400000 pts: 27.433333 pos:2674605 size:127383
+ret: 0         st: 0 flags:0 dts: 27.433333 pts: 27.466667 pos:2801988 size:    68
+ret: 0         st: 0 flags:0 dts: 27.466667 pts: 27.500000 pos:2802268 size:  1754
+ret: 0         st: 0 flags:0 dts: 27.500000 pts: 27.533333 pos:2804022 size:  4071
+ret: 0         st:-1 flags:1  ts: 128.200839
+ret: 0         st: 0 flags:1 dts: 127.833333 pts: 127.866667 pos:13514072 size: 67382
+ret: 0         st: 0 flags:0 dts: 127.866667 pts: 127.966667 pos:13581454 size:  2936
+ret: 0         st: 0 flags:0 dts: 127.900000 pts: 127.900000 pos:13584390 size:   451
+ret: 0         st: 0 flags:0 dts: 127.933333 pts: 127.933333 pos:13584841 size:   537
+ret: 0         st: 0 flags:0  ts: 51.095011
+ret: 0         st: 0 flags:1 dts: 52.033333 pts: 52.066667 pos:6028050 size:115809
+ret: 0         st: 0 flags:0 dts: 52.066667 pts: 52.166667 pos:6143859 size:  1620
+ret: 0         st: 0 flags:0 dts: 52.100000 pts: 52.100000 pos:6145479 size:    92
+ret: 0         st: 0 flags:0 dts: 52.133333 pts: 52.133333 pos:6145571 size:   533
+ret: 0         st: 0 flags:1  ts: 153.989178
+ret: 0         st: 0 flags:1 dts: 153.466667 pts: 153.500000 pos:15867700 size: 96169
+ret: 0         st: 0 flags:0 dts: 153.500000 pts: 153.533333 pos:15963869 size:   785
+ret: 0         st: 0 flags:0 dts: 153.533333 pts: 153.633333 pos:15964654 size:  3135
+ret: 0         st: 0 flags:0 dts: 153.566667 pts: 153.566667 pos:15967789 size:   859
+ret: 0         st:-1 flags:0  ts: 76.883340
+ret: 0         st: 0 flags:1 dts: 77.833333 pts: 77.866667 pos:8659657 size: 41182
+ret: 0         st: 0 flags:0 dts: 77.866667 pts: 77.966667 pos:8700839 size:  4197
+ret: 0         st: 0 flags:0 dts: 77.900000 pts: 77.900000 pos:8705036 size:   653
+ret: 0         st: 0 flags:0 dts: 77.933333 pts: 77.933333 pos:8705689 size:   751
+ret: 0         st:-1 flags:1  ts:-0.222493
+ret: 0         st: 0 flags:1 dts:-0.033333 pts: 0.000000 pos:   1287 size:   183
+ret: 0         st: 0 flags:0 dts: 0.000000 pts: 0.033333 pos:   1470 size:    24
+ret: 0         st: 0 flags:0 dts: 0.033333 pts: 0.066667 pos:   1494 size:  6779
+ret: 0         st: 0 flags:0 dts: 0.066667 pts: 0.100000 pos:   8273 size: 11041
+ret: 0         st: 0 flags:0  ts: 102.671678
+ret: 0         st: 0 flags:1 dts: 104.066667 pts: 104.100000 pos:11116461 size:112929
+ret: 0         st: 0 flags:0 dts: 104.100000 pts: 104.133333 pos:11229390 size:   585
+ret: 0         st: 0 flags:0 dts: 104.133333 pts: 104.166667 pos:11229975 size:   797
+ret: 0         st: 0 flags:0 dts: 104.166667 pts: 104.200000 pos:11230772 size:   810
+ret: 0         st: 0 flags:1  ts: 25.565844
+ret: 0         st: 0 flags:1 dts: 25.300000 pts: 25.333333 pos:2607246 size: 40273
+ret: 0         st: 0 flags:0 dts: 25.333333 pts: 25.433333 pos:2647519 size:  2959
+ret: 0         st: 0 flags:0 dts: 25.366667 pts: 25.366667 pos:2650478 size:   197
+ret: 0         st: 0 flags:0 dts: 25.400000 pts: 25.400000 pos:2650675 size:   230
+ret: 0         st:-1 flags:0  ts: 128.460008
+ret: 0         st: 0 flags:1 dts: 131.233333 pts: 131.266667 pos:13727953 size: 62229
+ret: 0         st: 0 flags:0 dts: 131.266667 pts: 131.366667 pos:13790182 size:  2349
+ret: 0         st: 0 flags:0 dts: 131.300000 pts: 131.300000 pos:13792531 size:   571
+ret: 0         st: 0 flags:0 dts: 131.333333 pts: 131.333333 pos:13793102 size:  1190
+ret: 0         st:-1 flags:1  ts: 51.354175
+ret: 0         st: 0 flags:1 dts: 50.600000 pts: 50.633333 pos:5858254 size: 67903
+ret: 0         st: 0 flags:0 dts: 50.633333 pts: 50.733333 pos:5926157 size:  1307
+ret: 0         st: 0 flags:0 dts: 50.666667 pts: 50.666667 pos:5927464 size:   150
+ret: 0         st: 0 flags:0 dts: 50.700000 pts: 50.700000 pos:5927614 size:   176
-- 
2.14.0.434.g98096fd7a8-goog