| Message ID | 20170804184648.11392-1-isasi@google.com |
|---|---|
| State | Superseded |
| Headers | show |
On 8/4/2017 7:46 PM, Sasi Inguva wrote: > According to https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFAppenG/QTFFAppenG.html > > 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(-) Is there a reference for ISOBMFF too? Because this changes mp4 output as well mov. > 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 > Can you confirm what exactly changes in these fragmented MP4 API tests? - Derek
From ISO-IEC-14496-12
*10 Sample Groups*
*10.1 Random Access Recovery Points*
*10.1.1.1*
*Definition*
*class AudioRollRecoveryEntry() extends AudioSampleGroupEntry (’roll’)*
*{*
*signed int(16) roll_distance;*
*}*
*An AudioRollRecoveryEntry documents the pre‐roll distance required in
audio streams in whichevery sample can be independently decoded, but the
decoder output is only assured to be correct afterpre‐rolling by the
indicated number of samples.*
*10.1.1.3roll_distance is a signed integer that gives the number of samples
that must be decoded inorder for a sample to be decoded correctly. A
positive value indicates the number of samplesafter the sample that is a
group member that must be decoded such that at the last of theserecovery is
complete, i.e. the last sample is correct. A negative value indicates the
number ofsamples before the sample that is a group member that must be
decoded in order for recoveryto be complete at the marked sample. The value
zero must not be used; the sync sample tabledocuments random access points
for which no recovery roll is needed.*
This patch sets the sgpd and sbgp atoms irrespective of whether it's
fragmented or not. So the byte addresses for some atoms will change for all
MP4 files .
On Fri, Aug 4, 2017 at 12:11 PM, Derek Buitenhuis <
derek.buitenhuis@gmail.com> wrote:
> On 8/4/2017 7:46 PM, Sasi Inguva wrote:
> > According to https://developer.apple.com/library/content/documentation/
> QuickTime/QTFF/QTFFAppenG/QTFFAppenG.html
> >
> > 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(-)
>
> Is there a reference for ISOBMFF too? Because this changes mp4 output as
> well mov.
>
> > 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
> >
>
> Can you confirm what exactly changes in these fragmented MP4 API tests?
>
> - Derek
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
On 8/4/2017 8:50 PM, Sasi Inguva wrote: > [...] Thanks for the reference, please add the section numbers alongside the QTFF doc in the commit message. > This patch sets the sgpd and sbgp atoms irrespective of whether it's > fragmented or not. So the byte addresses for some atoms will change for all > MP4 files . Sure. I just wanted to confirm that is all that's changing in those tests. - Derek
On 8/4/2017 7:46 PM, Sasi Inguva wrote: > According to https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFAppenG/QTFFAppenG.html > > 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 have been confirmed to be benign. - Derek
On 8/5/2017 5:27 PM, Derek Buitenhuis wrote:
> Probably OK, if FATE changes have been confirmed to be benign.
Woops, replied to the wrong patch version.
- Derek
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
According to https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFAppenG/QTFFAppenG.html 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(-)