[FFmpeg-devel] [PATCH 3/3] lavf/mp3dec: read encoder delay/padding from Info tag

[James Almer]

On 10/4/2016 12:52 PM, Hendrik Leppkes wrote:
> On Tue, Oct 4, 2016 at 2:45 AM, Jon Toohill
> <jtoohill-at-google.com at ffmpeg.org> wrote:
>> Muxers can check AVCodecParameters.initial_padding for the
>> encoder+decoder delay, and read the AV_PKT_DATA_SKIP_SAMPLES
>> side data from the last packet for the encoder padding.
>>
>> This change also fixes the first_discard_sample calculation
>> which erroneously included the decoder delay. Decoder delay
>> is already accounted for in st->skip_samples. The affected
>> FATE tests have been updated accordingly.
>> ---
>>  libavformat/mp3dec.c                 |  3 ++-
>>  tests/ref/fate/audiomatch-square-mp3 |  2 +-
>>  tests/ref/fate/gapless-mp3           | 10 +++++-----
>>  3 files changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>> index 56c7f8c..e8b2428 100644
>> --- a/libavformat/mp3dec.c
>> +++ b/libavformat/mp3dec.c
>> @@ -239,9 +239,10 @@ static void mp3_parse_info_tag(AVFormatContext *s, AVStream *st,
>>
>>          mp3->start_pad = v>>12;
>>          mp3->  end_pad = v&4095;
>> +        st->codecpar->initial_padding = mp3->start_pad + 528 + 1;
>>          st->start_skip_samples = mp3->start_pad + 528 + 1;
>>          if (mp3->frames) {
>> -            st->first_discard_sample = -mp3->end_pad + 528 + 1 + mp3->frames * (int64_t)spf;
>> +            st->first_discard_sample = -mp3->end_pad + mp3->frames * (int64_t)spf;
>>              st->last_discard_sample = mp3->frames * (int64_t)spf;
>>          }
>>          if (!st->start_time)
>> diff --git a/tests/ref/fate/audiomatch-square-mp3 b/tests/ref/fate/audiomatch-square-mp3
>> index 8de55c2..05176a0 100644
>> --- a/tests/ref/fate/audiomatch-square-mp3
>> +++ b/tests/ref/fate/audiomatch-square-mp3
>> @@ -1 +1 @@
>> -presig: 0 postsig:0 c: 0.9447 lenerr:0
>> +presig: 0 postsig:-529 c: 0.9334 lenerr:-529
>> diff --git a/tests/ref/fate/gapless-mp3 b/tests/ref/fate/gapless-mp3
>> index ebe7bfa..8b80bfc 100644
>> --- a/tests/ref/fate/gapless-mp3
>> +++ b/tests/ref/fate/gapless-mp3
>> @@ -1,5 +1,5 @@
>> -37534a3bcc3ef306e8c5ebfcfedfc41c *tests/data/fate/gapless-mp3.out-1
>> -c96c3ae7bd3300fd2f4debac222de5b7
>> -0cd1cdbcfd5cdbf6270cd98219bf31cd *tests/data/fate/gapless-mp3.out-2
>> -c96c3ae7bd3300fd2f4debac222de5b7
>> -9d3d8ba8a61b534f2d02ee648d6a8229 *tests/data/fate/gapless-mp3.out-3
>> +81695be427d45e8be4d527a6b2af2a85 *tests/data/fate/gapless-mp3.out-1
>> +c7879a827ab017364774069268d9a267
>> +62d074296f8c84a5f86a6afdd7bab459 *tests/data/fate/gapless-mp3.out-2
>> +c7879a827ab017364774069268d9a267
>> +e931f3fe1ba25e0d5eece4977c4061a9 *tests/data/fate/gapless-mp3.out-3
>> --
> 
> Presumably when these tests were setup, someone verified that their
> output was sane and proper, and gapless.
> So when these are changed, I have to ask - what exactly changes in
> this output? The hashes unfortunately don't tell us that.
> 
> - Hendrik

Changing the test to output the framecrc directly instead of doing a
md5sum of the output file shows this

TEST    gapless-mp3
--- /ffmpeg/src/tests/ref/fate/gapless-mp3      2016-10-04 13:08:51.271126400 -0300
+++ tests/data/fate/gapless-mp3 2016-10-04 13:09:26.519070600 -0300
@@ -596,7 +596,7 @@
 0,  217143996,  217143996,   368640,      418, 0xb260c6a6
 0,  217512636,  217512636,   368640,      418, 0xe448c368
 0,  217881276,  217881276,   368640,      418, 0xb229c63f
-0,  218249916,  218249916,   368640,      418, 0x53de9611, S=1,       10, 0x011f0030
+0,  218249916,  218249916,   368640,      418, 0x53de9611, S=1,       10, 0x018f0043
 0,  218618556,  218618556,   368640,      418, 0xe12f8514, S=1,       10, 0x03140084
 #tb 0: 1/44100
 #media_type 0: audio
@@ -1196,7 +1196,7 @@
 0,     678575,     678575,     1152,     4608, 0x5fd9abc4
 0,     679727,     679727,     1152,     4608, 0xc7ccda46
 0,     680879,     680879,     1152,     4608, 0x96c68e2f
-0,     682031,     682031,      849,     3396, 0x4fe14cc5
+0,     682031,     682031,      320,     1280, 0xb3535bc6
 #tb 0: 1/14112000
 #media_type 0: audio
 #codec_id 0: mp3
@@ -1795,7 +1795,7 @@
 0,  217497600,  217497600,   368640,      418, 0xb260c6a6
 0,  217866240,  217866240,   368640,      418, 0xe448c368
 0,  218234880,  218234880,   368640,      418, 0xb229c63f
-0,  218603520,  218603520,   368640,      418, 0x53de9611, S=1,       10, 0x011f0030
+0,  218603520,  218603520,   368640,      418, 0x53de9611, S=1,       10, 0x018f0043
 0,  218972160,  218972160,   368640,      418, 0xe12f8514, S=1,       10, 0x03140084
 #tb 0: 1/44100
 #media_type 0: audio
@@ -2395,7 +2395,7 @@
 0,     679680,     679680,     1152,     4608, 0x5fd9abc4
 0,     680832,     680832,     1152,     4608, 0xc7ccda46
 0,     681984,     681984,     1152,     4608, 0x96c68e2f
-0,     683136,     683136,      849,     3396, 0x4fe14cc5
+0,     683136,     683136,      320,     1280, 0xb3535bc6
 #tb 0: 1/14112000
 #media_type 0: audio
 #codec_id 0: mp3
@@ -2803,5 +2803,5 @@
 0,  146937600,  146937600,   368640,      418, 0xb260c6a6
 0,  147306240,  147306240,   368640,      418, 0xe448c368
 0,  147674880,  147674880,   368640,      418, 0xb229c63f
-0,  148043520,  148043520,   368640,      418, 0x53de9611, S=1,       10, 0x011f0030
+0,  148043520,  148043520,   368640,      418, 0x53de9611, S=1,       10, 0x018f0043
 0,  148412160,  148412160,   368640,      418, 0xe12f8514, S=1,       10, 0x03140084

What changed is the side data from one packet at the end for codec copy cases, and
what i think is duration in decoded cases.