[FFmpeg-devel] avformat/mov: prevent duplication of first fragment's ctts_data

Submitted by Daniel Glöckner on Aug. 6, 2017, 1:04 a.m.

Details

Message ID 1501981493-24031-1-git-send-email-daniel-gl@gmx.net
State Accepted
Commit feb1dbc7bd4c395400c48055c563ac23d5251716
Headers show

Commit Message

Daniel Glöckner Aug. 6, 2017, 1:04 a.m.
MP4 files with fragments might have the first moof box that is mentioned
in a fragment index before the first mdat box. Since it is then already
parsed by mov_read_header, we have to make sure that mov_switch_root
will not parse it again when seeking by setting the headers_read flag in
the index. Parsing it a second time would cause the ctts_data array to
receive a second copy of the information from the trun box, leading to
wrong PTS values for the second and following fragments in presence of
B-frames.

Fixes ticket 6560.

Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
---
 libavformat/mov.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Dale Curtis Aug. 28, 2017, 10:22 p.m.
lgtm, my patch is still necessary to ensure ctts samples aren't trampled
when new headers do need to be read. This should prevent unnecessary work
and spammy logs.

- dale

On Sat, Aug 5, 2017 at 6:04 PM, Daniel Glöckner <daniel-gl@gmx.net> wrote:

> MP4 files with fragments might have the first moof box that is mentioned
> in a fragment index before the first mdat box. Since it is then already
> parsed by mov_read_header, we have to make sure that mov_switch_root
> will not parse it again when seeking by setting the headers_read flag in
> the index. Parsing it a second time would cause the ctts_data array to
> receive a second copy of the information from the trun box, leading to
> wrong PTS values for the second and following fragments in presence of
> B-frames.
>
> Fixes ticket 6560.
>
> Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
> ---
>  libavformat/mov.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 63f84be..c2da1b6 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -6345,6 +6345,13 @@ static int mov_read_header(AVFormatContext *s)
>      }
>      ff_configure_buffers_for_index(s, AV_TIME_BASE);
>
> +    for (i = 0; i < mov->fragment_index_count; i++) {
> +        MOVFragmentIndex *idx = mov->fragment_index_data[i];
> +        for (j = 0; j < idx->item_count; j++)
> +            if (idx->items[j].moof_offset <= mov->fragment.moof_offset)
> +                idx->items[j].headers_read = 1;
> +    }
> +
>      return 0;
>  }
>
> --
> 2.7.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Michael Niedermayer Aug. 31, 2017, 7:36 p.m.
On Mon, Aug 28, 2017 at 03:22:43PM -0700, Dale Curtis wrote:
> lgtm, my patch is still necessary to ensure ctts samples aren't trampled
> when new headers do need to be read. This should prevent unnecessary work
> and spammy logs.

will apply

thanks

[...]

Patch hide | download patch | download mbox

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 63f84be..c2da1b6 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6345,6 +6345,13 @@  static int mov_read_header(AVFormatContext *s)
     }
     ff_configure_buffers_for_index(s, AV_TIME_BASE);
 
+    for (i = 0; i < mov->fragment_index_count; i++) {
+        MOVFragmentIndex *idx = mov->fragment_index_data[i];
+        for (j = 0; j < idx->item_count; j++)
+            if (idx->items[j].moof_offset <= mov->fragment.moof_offset)
+                idx->items[j].headers_read = 1;
+    }
+
     return 0;
 }