diff mbox

[FFmpeg-devel] lavf/mov.c: Parse upto 2 keyframes after the edit list end in mov_fix_index.

Message ID 20171101212645.12284-1-isasi@google.com
State Superseded
Headers show

Commit Message

Sasi Inguva Nov. 1, 2017, 9:26 p.m. UTC
Partially fixes t/6699.
---
 libavformat/mov.c                         | 32 ++++++++++++++++++++-----------
 tests/fate/mov.mak                        |  4 ++++
 tests/ref/fate/mov-elst-ends-betn-b-and-i | 31 ++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 11 deletions(-)
 create mode 100644 tests/ref/fate/mov-elst-ends-betn-b-and-i

Comments

Sasi Inguva Nov. 1, 2017, 9:27 p.m. UTC | #1
Pls find attached, the FATE sample.


On Wed, Nov 1, 2017 at 2:26 PM, Sasi Inguva <isasi@google.com> wrote:

> Partially fixes t/6699.
> ---
>  libavformat/mov.c                         | 32
> ++++++++++++++++++++-----------
>  tests/fate/mov.mak                        |  4 ++++
>  tests/ref/fate/mov-elst-ends-betn-b-and-i | 31
> ++++++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+), 11 deletions(-)
>  create mode 100644 tests/ref/fate/mov-elst-ends-betn-b-and-i
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 60f0228e2d..e716e5ba9e 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -3232,6 +3232,7 @@ static void mov_fix_index(MOVContext *mov, AVStream
> *st)
>      int packet_skip_samples = 0;
>      MOVIndexRange *current_index_range;
>      int i;
> +    int found_keyframe_after_edit = 0;
>
>      if (!msc->elst_data || msc->elst_count <= 0 || nb_old <= 0) {
>          return;
> @@ -3347,6 +3348,7 @@ static void mov_fix_index(MOVContext *mov, AVStream
> *st)
>
>          // Iterate over index and arrange it according to edit list
>          edit_list_start_encountered = 0;
> +        found_keyframe_after_edit = 0;
>          for (; current < e_old_end; current++, index++) {
>              // check  if frame outside edit list mark it for discard
>              frame_duration = (current + 1 <  e_old_end) ?
> @@ -3459,18 +3461,26 @@ static void mov_fix_index(MOVContext *mov,
> AVStream *st)
>              }
>
>              // Break when found first key frame after edit entry
> completion
> -            if (((curr_cts + frame_duration) >= (edit_list_duration +
> edit_list_media_time)) &&
> +            if ((curr_cts + frame_duration >= (edit_list_duration +
> edit_list_media_time)) &&
>                  ((flags & AVINDEX_KEYFRAME) || ((st->codecpar->codec_type
> == AVMEDIA_TYPE_AUDIO)))) {
> -
> -                if (ctts_data_old && ctts_sample_old != 0) {
> -                    if (add_ctts_entry(&msc->ctts_data, &msc->ctts_count,
> -                                       &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",
> -                               ctts_index_old, ctts_sample_old -
> edit_list_start_ctts_sample,
> -                               ctts_data_old[ctts_index_old].duration);
> -                        break;
> +                if (ctts_data_old) {
> +                    // If we have CTTS and this is the the first keyframe
> after edit elist,
> +                    // wait for one more, because there might be trailing
> B-frames after this I-frame
> +                    // that do belong to the edit.
> +                    if (st->codecpar->codec_type != AVMEDIA_TYPE_AUDIO &&
> found_keyframe_after_edit == 0) {
> +                        found_keyframe_after_edit = 1;
> +                        continue;
> +                    }
> +                    if (ctts_sample_old != 0) {
> +                        if (add_ctts_entry(&msc->ctts_data,
> &msc->ctts_count,
> +                                           &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",
> +                                   ctts_index_old, ctts_sample_old -
> edit_list_start_ctts_sample,
> +                                   ctts_data_old[ctts_index_old].
> duration);
> +                            break;
> +                        }
>                      }
>                  }
>                  break;
> diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
> index 6815e4feca..e444b1459d 100644
> --- a/tests/fate/mov.mak
> +++ b/tests/fate/mov.mak
> @@ -9,6 +9,7 @@ FATE_MOV = fate-mov-3elist \
>             fate-mov-invalid-elst-entry-count \
>             fate-mov-gpmf-remux \
>             fate-mov-440hz-10ms \
> +           fate-mov-elst-ends-betn-b-and-i \
>
>  FATE_MOV_FFPROBE = fate-mov-aac-2048-priming \
>                     fate-mov-zombie \
> @@ -41,6 +42,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 if edit list ends on a B-frame but before the I-frame,
> then we output the B-frame but discard the I-frame.
> +fate-mov-elst-ends-betn-b-and-i: CMD = framemd5 -i
> $(TARGET_SAMPLES)/mov/elst_ends_betn_b_and_i.mp4
> +
>  # Makes sure that we handle edit lists and start padding correctly.
>  fate-mov-440hz-10ms: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/440hz-
> 10ms.m4a
>
> diff --git a/tests/ref/fate/mov-elst-ends-betn-b-and-i
> b/tests/ref/fate/mov-elst-ends-betn-b-and-i
> new file mode 100644
> index 0000000000..ee7c6389ad
> --- /dev/null
> +++ b/tests/ref/fate/mov-elst-ends-betn-b-and-i
> @@ -0,0 +1,31 @@
> +#format: frame checksums
> +#version: 2
> +#hash: MD5
> +#tb 0: 1/25
> +#media_type 0: video
> +#codec_id 0: rawvideo
> +#dimensions 0: 320x240
> +#sar 0: 1/1
> +#stream#, dts,        pts, duration,     size, hash
> +0,          0,          0,        1,   115200,
> 4e5dc2b806e394cd666c968f736fecd0
> +0,          1,          1,        1,   115200,
> 7a3c7473d44c5f60c07655f6fc0c2ac3
> +0,          2,          2,        1,   115200,
> 038254422a603a3270c09cdcd149707b
> +0,          3,          3,        1,   115200,
> 7553b6b4547cb23ef8f0392ed5a5d4b0
> +0,          4,          4,        1,   115200,
> 6d017ede7f446124af7308667cb0dc41
> +0,          5,          5,        1,   115200,
> 77752f0288ae64f857732b8e62e47457
> +0,          6,          6,        1,   115200,
> d656833951af99330625f7c6de7685c4
> +0,          7,          7,        1,   115200,
> 14338b833e431e566ac98da841600bfe
> +0,          8,          8,        1,   115200,
> 07ea95d1659f3c4424a470a546d0df6e
> +0,          9,          9,        1,   115200,
> fd05b8cc83072f813e89d394d1f6efc6
> +0,         10,         10,        1,   115200,
> 750b82ca5c7e901545e7b1aa69692426
> +0,         11,         11,        1,   115200,
> 7347679ab09bc936047368b8caebcaff
> +0,         12,         12,        1,   115200,
> 63a23fdd57ac8462b9ffbcb12ab717b3
> +0,         13,         13,        1,   115200,
> 705257a1c99693db233e2a3ee027adcf
> +0,         14,         14,        1,   115200,
> df861a2ec7a4ef70e82b1c28025e5a48
> +0,         15,         15,        1,   115200,
> 2a8b403c077b6b43aa71eaf7d1537713
> +0,         16,         16,        1,   115200,
> 973b5cd3ce473e3970dfa96045553172
> +0,         17,         17,        1,   115200,
> fc612c0afeae3b6576b5ee2f3f119832
> +0,         18,         18,        1,   115200,
> 97074fe5a0b6e7e8470729654092e56c
> +0,         19,         19,        1,   115200,
> 8cf9337201065335b3aa4da21dc9b37a
> +0,         20,         20,        1,   115200,
> 93ff3589294cc0673af3daee1e7fe42a
> +0,         21,         21,        1,   115200,
> c0b6fd870a022f374f9d6c697e8e293d
> --
> 2.15.0.403.gc27cc4dac6-goog
>
>
Michael Niedermayer Nov. 2, 2017, 11:23 p.m. UTC | #2
On Wed, Nov 01, 2017 at 02:27:50PM -0700, Sasi Inguva wrote:
> Pls find attached, the FATE sample.

uplaoded

[...]
Sasi Inguva Nov. 9, 2017, 12:13 a.m. UTC | #3
Updated the fate test after
http://git.videolan.org/?p=ffmpeg.git;a=commit;h=c2a8f0fcbe57ea9ccaa864130f078af10516c3c1
. Sending the updated patch.

On Thu, Nov 2, 2017 at 4:23 PM, Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Wed, Nov 01, 2017 at 02:27:50PM -0700, Sasi Inguva wrote:
> > Pls find attached, the FATE sample.
>
> uplaoded
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> There will always be a question for which you do not know the correct
> answer.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
diff mbox

Patch

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 60f0228e2d..e716e5ba9e 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3232,6 +3232,7 @@  static void mov_fix_index(MOVContext *mov, AVStream *st)
     int packet_skip_samples = 0;
     MOVIndexRange *current_index_range;
     int i;
+    int found_keyframe_after_edit = 0;
 
     if (!msc->elst_data || msc->elst_count <= 0 || nb_old <= 0) {
         return;
@@ -3347,6 +3348,7 @@  static void mov_fix_index(MOVContext *mov, AVStream *st)
 
         // Iterate over index and arrange it according to edit list
         edit_list_start_encountered = 0;
+        found_keyframe_after_edit = 0;
         for (; current < e_old_end; current++, index++) {
             // check  if frame outside edit list mark it for discard
             frame_duration = (current + 1 <  e_old_end) ?
@@ -3459,18 +3461,26 @@  static void mov_fix_index(MOVContext *mov, AVStream *st)
             }
 
             // Break when found first key frame after edit entry completion
-            if (((curr_cts + frame_duration) >= (edit_list_duration + edit_list_media_time)) &&
+            if ((curr_cts + frame_duration >= (edit_list_duration + edit_list_media_time)) &&
                 ((flags & AVINDEX_KEYFRAME) || ((st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO)))) {
-
-                if (ctts_data_old && ctts_sample_old != 0) {
-                    if (add_ctts_entry(&msc->ctts_data, &msc->ctts_count,
-                                       &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",
-                               ctts_index_old, ctts_sample_old - edit_list_start_ctts_sample,
-                               ctts_data_old[ctts_index_old].duration);
-                        break;
+                if (ctts_data_old) {
+                    // If we have CTTS and this is the the first keyframe after edit elist,
+                    // wait for one more, because there might be trailing B-frames after this I-frame
+                    // that do belong to the edit.
+                    if (st->codecpar->codec_type != AVMEDIA_TYPE_AUDIO && found_keyframe_after_edit == 0) {
+                        found_keyframe_after_edit = 1;
+                        continue;
+                    }
+                    if (ctts_sample_old != 0) {
+                        if (add_ctts_entry(&msc->ctts_data, &msc->ctts_count,
+                                           &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",
+                                   ctts_index_old, ctts_sample_old - edit_list_start_ctts_sample,
+                                   ctts_data_old[ctts_index_old].duration);
+                            break;
+                        }
                     }
                 }
                 break;
diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
index 6815e4feca..e444b1459d 100644
--- a/tests/fate/mov.mak
+++ b/tests/fate/mov.mak
@@ -9,6 +9,7 @@  FATE_MOV = fate-mov-3elist \
            fate-mov-invalid-elst-entry-count \
            fate-mov-gpmf-remux \
            fate-mov-440hz-10ms \
+           fate-mov-elst-ends-betn-b-and-i \
 
 FATE_MOV_FFPROBE = fate-mov-aac-2048-priming \
                    fate-mov-zombie \
@@ -41,6 +42,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 if edit list ends on a B-frame but before the I-frame, then we output the B-frame but discard the I-frame.
+fate-mov-elst-ends-betn-b-and-i: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/elst_ends_betn_b_and_i.mp4
+
 # Makes sure that we handle edit lists and start padding correctly.
 fate-mov-440hz-10ms: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/440hz-10ms.m4a
 
diff --git a/tests/ref/fate/mov-elst-ends-betn-b-and-i b/tests/ref/fate/mov-elst-ends-betn-b-and-i
new file mode 100644
index 0000000000..ee7c6389ad
--- /dev/null
+++ b/tests/ref/fate/mov-elst-ends-betn-b-and-i
@@ -0,0 +1,31 @@ 
+#format: frame checksums
+#version: 2
+#hash: MD5
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 320x240
+#sar 0: 1/1
+#stream#, dts,        pts, duration,     size, hash
+0,          0,          0,        1,   115200, 4e5dc2b806e394cd666c968f736fecd0
+0,          1,          1,        1,   115200, 7a3c7473d44c5f60c07655f6fc0c2ac3
+0,          2,          2,        1,   115200, 038254422a603a3270c09cdcd149707b
+0,          3,          3,        1,   115200, 7553b6b4547cb23ef8f0392ed5a5d4b0
+0,          4,          4,        1,   115200, 6d017ede7f446124af7308667cb0dc41
+0,          5,          5,        1,   115200, 77752f0288ae64f857732b8e62e47457
+0,          6,          6,        1,   115200, d656833951af99330625f7c6de7685c4
+0,          7,          7,        1,   115200, 14338b833e431e566ac98da841600bfe
+0,          8,          8,        1,   115200, 07ea95d1659f3c4424a470a546d0df6e
+0,          9,          9,        1,   115200, fd05b8cc83072f813e89d394d1f6efc6
+0,         10,         10,        1,   115200, 750b82ca5c7e901545e7b1aa69692426
+0,         11,         11,        1,   115200, 7347679ab09bc936047368b8caebcaff
+0,         12,         12,        1,   115200, 63a23fdd57ac8462b9ffbcb12ab717b3
+0,         13,         13,        1,   115200, 705257a1c99693db233e2a3ee027adcf
+0,         14,         14,        1,   115200, df861a2ec7a4ef70e82b1c28025e5a48
+0,         15,         15,        1,   115200, 2a8b403c077b6b43aa71eaf7d1537713
+0,         16,         16,        1,   115200, 973b5cd3ce473e3970dfa96045553172
+0,         17,         17,        1,   115200, fc612c0afeae3b6576b5ee2f3f119832
+0,         18,         18,        1,   115200, 97074fe5a0b6e7e8470729654092e56c
+0,         19,         19,        1,   115200, 8cf9337201065335b3aa4da21dc9b37a
+0,         20,         20,        1,   115200, 93ff3589294cc0673af3daee1e7fe42a
+0,         21,         21,        1,   115200, c0b6fd870a022f374f9d6c697e8e293d