[FFmpeg-devel] lavf/movenc.c: Set sgpd and sbgp atoms to represent decoder delay for AAC.

Submitted by Sasi Inguva on Aug. 4, 2017, 8:35 p.m.

Details

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

Commit Message

Sasi Inguva Aug. 4, 2017, 8:35 p.m.
According to https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFAppenG/QTFFAppenG.html and ISO-IEC-14496-12 Section 10.1.1.1 and 10.1.1.3

Signed-off-by: Sasi Inguva <isasi@google.com>
---
 libavformat/movenc.c                | 70 +++++++++++++++++++++----------------
 tests/ref/fate/adtstoasc_ticket3715 |  4 +--
 tests/ref/fate/copy-psp             |  4 +--
 tests/ref/fate/movenc               | 12 +++----
 4 files changed, 49 insertions(+), 41 deletions(-)

Comments

Derek Buitenhuis Aug. 5, 2017, 4:27 p.m.
On 8/4/2017 9:35 PM, Sasi Inguva wrote:
> According to https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFAppenG/QTFFAppenG.html and ISO-IEC-14496-12 Section 10.1.1.1 and 10.1.1.3
> 
> Signed-off-by: Sasi Inguva <isasi@google.com>
> ---
>  libavformat/movenc.c                | 70 +++++++++++++++++++++----------------
>  tests/ref/fate/adtstoasc_ticket3715 |  4 +--
>  tests/ref/fate/copy-psp             |  4 +--
>  tests/ref/fate/movenc               | 12 +++----
>  4 files changed, 49 insertions(+), 41 deletions(-)

Probably OK, if FATE changes are benign.

- Derek
Sasi Inguva Aug. 5, 2017, 5:08 p.m.
To the best of my knowledge, all fate changes are benign. All of them
correspond to a size increase of 54 bytes, which is the total size of the
new atoms added.

On Sat, Aug 5, 2017 at 9:27 AM, Derek Buitenhuis <derek.buitenhuis@gmail.com
> wrote:

> On 8/4/2017 9:35 PM, Sasi Inguva wrote:
> > According to https://developer.apple.com/library/content/documentation/
> QuickTime/QTFF/QTFFAppenG/QTFFAppenG.html and ISO-IEC-14496-12 Section
> 10.1.1.1 and 10.1.1.3
> >
> > Signed-off-by: Sasi Inguva <isasi@google.com>
> > ---
> >  libavformat/movenc.c                | 70 +++++++++++++++++++++---------
> -------
> >  tests/ref/fate/adtstoasc_ticket3715 |  4 +--
> >  tests/ref/fate/copy-psp             |  4 +--
> >  tests/ref/fate/movenc               | 12 +++----
> >  4 files changed, 49 insertions(+), 41 deletions(-)
>
> Probably OK, if FATE changes are benign.
>
> - Derek
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Sasi Inguva Aug. 8, 2017, 8:55 p.m.
Ping!

On Aug 5, 2017 10:08 AM, "Sasi Inguva" <isasi@google.com> wrote:

> To the best of my knowledge, all fate changes are benign. All of them
> correspond to a size increase of 54 bytes, which is the total size of the
> new atoms added.
>
> On Sat, Aug 5, 2017 at 9:27 AM, Derek Buitenhuis <
> derek.buitenhuis@gmail.com> wrote:
>
>> On 8/4/2017 9:35 PM, Sasi Inguva wrote:
>> > According to https://developer.apple.com/li
>> brary/content/documentation/QuickTime/QTFF/QTFFAppenG/QTFFAppenG.html
>> and ISO-IEC-14496-12 Section 10.1.1.1 and 10.1.1.3
>> >
>> > Signed-off-by: Sasi Inguva <isasi@google.com>
>> > ---
>> >  libavformat/movenc.c                | 70 +++++++++++++++++++++---------
>> -------
>> >  tests/ref/fate/adtstoasc_ticket3715 |  4 +--
>> >  tests/ref/fate/copy-psp             |  4 +--
>> >  tests/ref/fate/movenc               | 12 +++----
>> >  4 files changed, 49 insertions(+), 41 deletions(-)
>>
>> Probably OK, if FATE changes are benign.
>>
>> - Derek
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
>
Carl Eugen Hoyos Aug. 8, 2017, 9:18 p.m.
Hi!

> Am 08.08.2017 um 22:55 schrieb Sasi Inguva <isasi-at-google.com@ffmpeg.org>:
> 
> Ping!

Please send a patch without the cosmetic changes to allow reviewing.

Carl Eugen

Patch hide | download patch | download mbox

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 0e5b45d150..10b959ad02 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2208,39 +2208,47 @@  static int mov_preroll_write_stbl_atoms(AVIOContext *pb, MOVTrack *track)
                                     (AVRational){1, 1000},
                                     (AVRational){1, 48000});
 
-    if (track->entry) {
-        sgpd_entries = av_malloc_array(track->entry, sizeof(*sgpd_entries));
-        if (!sgpd_entries)
-            return AVERROR(ENOMEM);
-    }
+    if (!track->entry)
+        return 0;
 
-    av_assert0(track->par->codec_id == AV_CODEC_ID_OPUS);
+    sgpd_entries = av_malloc_array(track->entry, sizeof(*sgpd_entries));
+    if (!sgpd_entries)
+        return AVERROR(ENOMEM);
 
-    for (i = 0; i < track->entry; i++) {
-        int roll_samples_remaining = roll_samples;
-        int distance = 0;
-        for (j = i - 1; j >= 0; j--) {
-            roll_samples_remaining -= get_cluster_duration(track, j);
-            distance++;
-            if (roll_samples_remaining <= 0)
-                break;
-        }
-        /* We don't have enough preceeding samples to compute a valid
-           roll_distance here, so this sample can't be independently
-           decoded. */
-        if (roll_samples_remaining > 0)
-            distance = 0;
-        /* Verify distance is a minimum of 2 (60ms) packets and a maximum of
-           32 (2.5ms) packets. */
-        av_assert0(distance == 0 || (distance >= 2 && distance <= 32));
-        if (i && distance == sgpd_entries[entries].roll_distance) {
-            sgpd_entries[entries].count++;
-        } else {
-            entries++;
-            sgpd_entries[entries].count = 1;
-            sgpd_entries[entries].roll_distance = distance;
-            sgpd_entries[entries].group_description_index = distance ? ++group : 0;
+    av_assert0(track->par->codec_id == AV_CODEC_ID_OPUS || track->par->codec_id == AV_CODEC_ID_AAC);
+
+    if (track->par->codec_id == AV_CODEC_ID_OPUS) {
+        for (i = 0; i < track->entry; i++) {
+            int roll_samples_remaining = roll_samples;
+            int distance = 0;
+            for (j = i - 1; j >= 0; j--) {
+                roll_samples_remaining -= get_cluster_duration(track, j);
+                distance++;
+                if (roll_samples_remaining <= 0)
+                    break;
+            }
+            /* We don't have enough preceeding samples to compute a valid
+               roll_distance here, so this sample can't be independently
+               decoded. */
+            if (roll_samples_remaining > 0)
+                distance = 0;
+            /* Verify distance is a minimum of 2 (60ms) packets and a maximum of
+               32 (2.5ms) packets. */
+            av_assert0(distance == 0 || (distance >= 2 && distance <= 32));
+            if (i && distance == sgpd_entries[entries].roll_distance) {
+                sgpd_entries[entries].count++;
+            } else {
+                entries++;
+                sgpd_entries[entries].count = 1;
+                sgpd_entries[entries].roll_distance = distance;
+                sgpd_entries[entries].group_description_index = distance ? ++group : 0;
+            }
         }
+    } else {
+        entries++;
+        sgpd_entries[entries].count = track->sample_count;
+        sgpd_entries[entries].roll_distance = 1;
+        sgpd_entries[entries].group_description_index = ++group;
     }
     entries++;
 
@@ -2304,7 +2312,7 @@  static int mov_write_stbl_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext
     if (track->cenc.aes_ctr) {
         ff_mov_cenc_write_stbl_atoms(&track->cenc, pb);
     }
-    if (track->par->codec_id == AV_CODEC_ID_OPUS) {
+    if (track->par->codec_id == AV_CODEC_ID_OPUS || track->par->codec_id == AV_CODEC_ID_AAC) {
         mov_preroll_write_stbl_atoms(pb, track);
     }
     return update_size(pb, pos);
diff --git a/tests/ref/fate/adtstoasc_ticket3715 b/tests/ref/fate/adtstoasc_ticket3715
index 949b565c2f..96795a2ca3 100644
--- a/tests/ref/fate/adtstoasc_ticket3715
+++ b/tests/ref/fate/adtstoasc_ticket3715
@@ -1,5 +1,5 @@ 
-ef8ce3cbd1d86113e7c991a816086068 *tests/data/fate/adtstoasc_ticket3715.mov
-33270 tests/data/fate/adtstoasc_ticket3715.mov
+0221e04333e6ac432fa42960502f0d5a *tests/data/fate/adtstoasc_ticket3715.mov
+33324 tests/data/fate/adtstoasc_ticket3715.mov
 #extradata 0:        2, 0x00340022
 #tb 0: 1/44100
 #media_type 0: audio
diff --git a/tests/ref/fate/copy-psp b/tests/ref/fate/copy-psp
index 6603d3ff26..81eb172549 100644
--- a/tests/ref/fate/copy-psp
+++ b/tests/ref/fate/copy-psp
@@ -1,5 +1,5 @@ 
-6889223644fc560069c8591984175a62 *tests/data/fate/copy-psp.psp
-2041379 tests/data/fate/copy-psp.psp
+cada61453a2483ef8ba1fb82c8bbff25 *tests/data/fate/copy-psp.psp
+2041433 tests/data/fate/copy-psp.psp
 #extradata 0:       51, 0xaf6d1012
 #extradata 1:        2, 0x00b200a1
 #tb 0: 1/90000
diff --git a/tests/ref/fate/movenc b/tests/ref/fate/movenc
index 09e603aeb7..47bcf9d515 100644
--- a/tests/ref/fate/movenc
+++ b/tests/ref/fate/movenc
@@ -1,18 +1,18 @@ 
 write_data len 36, time nopts, type header atom ftyp
-write_data len 2335, time nopts, type header atom -
+write_data len 2389, time nopts, type header atom -
 write_data len 788, time 1000000, type sync atom moof
 write_data len 110, time nopts, type trailer atom -
-214242e9c7c93171d2f47f5b47776559 3269 non-empty-moov
+17a37691eba8b858cf15e60aa9a7dbf7 3323 non-empty-moov
 write_data len 36, time nopts, type header atom ftyp
-write_data len 2667, time nopts, type header atom -
+write_data len 2721, time nopts, type header atom -
 write_data len 908, time 966667, type sync atom moof
 write_data len 110, time nopts, type trailer atom -
-44467d568a3cc38d414fd8ed4b2a968f 3721 non-empty-moov-elst
+0026ffe059c06c592021f972bf2c5e79 3775 non-empty-moov-elst
 write_data len 36, time nopts, type header atom ftyp
-write_data len 2575, time nopts, type header atom -
+write_data len 2629, time nopts, type header atom -
 write_data len 908, time 1000000, type sync atom moof
 write_data len 110, time nopts, type trailer atom -
-de22b98a3885f9b4b83cdd48ff46aeb9 3629 non-empty-moov-no-elst
+c184e168ac1e5bb3d9c70e580ab6179c 3683 non-empty-moov-no-elst
 write_data len 20, time nopts, type header atom ftyp
 write_data len 1171, time nopts, type header atom -
 write_data len 728, time 0, type sync atom moof