diff mbox

[FFmpeg-devel,mov] When both edit list and start padding present, take maximum.

Message ID CAPUDrwcFvbQRJxMJkJvaFnELuUpex1wV1umm7j2T5FAS0d=Sog@mail.gmail.com
State New
Headers show

Commit Message

Dale Curtis Oct. 16, 2017, 9:22 p.m. UTC 
Previously the start padding was used to blindly overwrite any skip samples
which may have come from an edit list. Instead take the maximum of the two.

A new fate test is added, fate-mov-440hz-10ms, to ensure this is handled
correctly.

The sample can be downloaded and added to the fate-suite from

https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/webaudio/resources/media/440hz-10ms.m4a

- dale

Comments

Dale Curtis Oct. 16, 2017, 9:25 p.m. UTC | #1 
More details on the issue which uncovered this can be seen here
https://bugs.chromium.org/p/chromium/issues/detail?id=775042#c13

- dale

On Mon, Oct 16, 2017 at 2:22 PM, Dale Curtis <dalecurtis@chromium.org>
wrote:

> Previously the start padding was used to blindly overwrite any skip
> samples which may have come from an edit list. Instead take the maximum of
> the two.
>
> A new fate test is added, fate-mov-440hz-10ms, to ensure this is handled
> correctly.
>
> The sample can be downloaded and added to the fate-suite from
>
> https://cs.chromium.org/chromium/src/third_party/
> WebKit/LayoutTests/webaudio/resources/media/440hz-10ms.m4a
>
> - dale
>
Michael Niedermayer Oct. 18, 2017, 12:46 a.m. UTC | #2 
On Mon, Oct 16, 2017 at 02:22:45PM -0700, Dale Curtis wrote:
> Previously the start padding was used to blindly overwrite any skip samples
> which may have come from an edit list. Instead take the maximum of the two.
> 
> A new fate test is added, fate-mov-440hz-10ms, to ensure this is handled
> correctly.
> 
> The sample can be downloaded and added to the fate-suite from
> 

> https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/webaudio/resources/media/440hz-10ms.m4a

sample uploaded to fatesamples

[...]
diff mbox

Patch

From 4d241904bc9f923f3c9cd93f19daae08bafbdae1 Mon Sep 17 00:00:00 2001
From: Dale Curtis <dalecurtis@chromium.org>
Date: Mon, 16 Oct 2017 14:17:35 -0700
Subject: [PATCH] [mov] When both edit list and start padding present, take
 maximum.

Previously the start padding was used to blindly overwrite any
skip samples which may have come from an edit list. Instead take
the maximum of the two.

A new fate test is added, fate-mov-440hz-10ms, to ensure this is
handled correctly.

Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
---
 libavformat/mov.c             |  3 ++-
 tests/fate/mov.mak            |  4 ++++
 tests/ref/fate/mov-440hz-10ms | 11 +++++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 tests/ref/fate/mov-440hz-10ms

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 899690d920..097f6a20dc 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6244,7 +6244,8 @@  static int mov_read_header(AVFormatContext *s)
         MOVStreamContext *sc = st->priv_data;
         fix_timescale(mov, sc);
         if(st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && st->codecpar->codec_id == AV_CODEC_ID_AAC) {
-            st->skip_samples = sc->start_pad;
+            /* skip samples may have already been injected by an edit list. */
+            st->skip_samples = FFMAX(st->skip_samples, sc->start_pad);
         }
         if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && sc->nb_frames_for_fps > 0 && sc->duration_for_fps > 0)
             av_reduce(&st->avg_frame_rate.num, &st->avg_frame_rate.den,
diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
index cfdada7a2e..5013e7d528 100644
--- a/tests/fate/mov.mak
+++ b/tests/fate/mov.mak
@@ -7,6 +7,7 @@  FATE_MOV = fate-mov-3elist \
            fate-mov-2elist-elist1-ends-bframe \
            fate-mov-3elist-encrypted \
            fate-mov-gpmf-remux \
+           fate-mov-440hz-10ms \
 
 FATE_MOV_FFPROBE = fate-mov-aac-2048-priming \
                    fate-mov-zombie \
@@ -39,6 +40,9 @@  fate-mov-1elist-ends-last-bframe: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/mov-1e
 # Makes sure that we handle timestamps of packets in case of multiple edit lists with one of them ending on a B-frame correctly.
 fate-mov-2elist-elist1-ends-bframe: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/mov-2elist-elist1-ends-bframe.mov
 
+# Makes sure that we handle edit lists and start padding correctly.
+fate-mov-440hz-10ms: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/440hz-10ms.m4a
+
 fate-mov-aac-2048-priming: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_packets -print_format compact $(TARGET_SAMPLES)/mov/aac-2048-priming.mov
 
 fate-mov-zombie: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_streams -show_packets -show_frames -bitexact -print_format compact $(TARGET_SAMPLES)/mov/white_zombie_scrunch-part.mov
diff --git a/tests/ref/fate/mov-440hz-10ms b/tests/ref/fate/mov-440hz-10ms
new file mode 100644
index 0000000000..498879e52d
--- /dev/null
+++ b/tests/ref/fate/mov-440hz-10ms
@@ -0,0 +1,11 @@ 
+#format: frame checksums
+#version: 2
+#hash: MD5
+#tb 0: 1/44100
+#media_type 0: audio
+#codec_id 0: pcm_s16le
+#sample_rate 0: 44100
+#channel_layout 0: 4
+#channel_layout_name 0: mono
+#stream#, dts,        pts, duration,     size, hash
+0,          0,          0,      960,     1920, 44e7e48ff08835ce30e93c7971dae7df
-- 
2.15.0.rc0.271.g36b669edcc-goog