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

2016-10-05 Thread Jon Toohill 
On Wed, Oct 5, 2016 at 11:03 AM, wm4  wrote:

> On Wed, 5 Oct 2016 10:42:13 -0700
> Jon Toohill  wrote:
>
> > On Wed, Oct 5, 2016 at 10:40 AM, Jon Toohill 
> wrote:
> >
> > > On Tue, Oct 4, 2016 at 7:19 AM, wm4  wrote:
> > >
> > >> On Mon,  3 Oct 2016 17:45:08 -0700
> > >> Jon Toohill  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;
> > >>
> > >> How does mixing these even make sense?
> > >>
> > >
> > > mp3enc.c already uses initial_padding for the encoder delay, and as you
> > > previously pointed out, mp3dec.c already uses
> AV_PKT_START_SKIP_SAMPLES for
> > > the encoder delay. I'm not attempting to solve the inconsistency in
> this
> > > patch set.
> > >
> >
> > err, *mp3dec.c already uses AV_PKT_DATA_SKIP_SAMPLES for the encoder
> > padding. Sorry for the confusion.
> >
>
> Not solving the inconsistency is problematic - but the worse issue is
> that you seem to introduce inconsistencies. In my current opinion, the
> packet side data and the initial_padding fields do the same (i.e.
> duplicated API), so only one of them can or should be used. Your patch
> seems to require the decoder to use them both, though.
>

That's a fair point; I'll send a revised patch set that doesn't change
mp3dec.c.

Note that I don't think I can get away from having libmp3lame.c set
AVCodecContext.initial_padding, since that field is used by the
AudioFrameQueue (and other encoders rely on it as well).

___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-10-05 Thread wm4 
On Wed, 5 Oct 2016 10:42:13 -0700
Jon Toohill  wrote:

> On Wed, Oct 5, 2016 at 10:40 AM, Jon Toohill  wrote:
> 
> > On Tue, Oct 4, 2016 at 7:19 AM, wm4  wrote:
> >  
> >> On Mon,  3 Oct 2016 17:45:08 -0700
> >> Jon Toohill  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;
> >>
> >> How does mixing these even make sense?
> >>  
> >
> > mp3enc.c already uses initial_padding for the encoder delay, and as you
> > previously pointed out, mp3dec.c already uses AV_PKT_START_SKIP_SAMPLES for
> > the encoder delay. I'm not attempting to solve the inconsistency in this
> > patch set.
> >  
> 
> err, *mp3dec.c already uses AV_PKT_DATA_SKIP_SAMPLES for the encoder
> padding. Sorry for the confusion.
> 

Not solving the inconsistency is problematic - but the worse issue is
that you seem to introduce inconsistencies. In my current opinion, the
packet side data and the initial_padding fields do the same (i.e.
duplicated API), so only one of them can or should be used. Your patch
seems to require the decoder to use them both, though.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-10-05 Thread Jon Toohill 
On Tue, Oct 4, 2016 at 9:10 AM, Michael Niedermayer 
wrote:

> On Mon, Oct 03, 2016 at 05:45:08PM -0700, Jon Toohill 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
>
> isnt this exactly correct before and wrong after this change ?
>
> zero signal before and zero signal after the original is what one would
> expect and equal length and high correlation
>
> every number that changes gets worse
>

Michael - you're right, this patch is incorrect currently. I had mistakenly
convinced myself that mp3dec.c was overcompensating for the decoder delay.

I also found that LAME itself writes the encoder padding + decoder delay
into the trailing padding field (e.g. for an input where 468 samples of
padding are added by the encoder, the Info tag contains the value 997).
I'll send a revised patch set.


> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Never trust a computer, one day, it may think you are the virus. -- Compn
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-10-05 Thread Jon Toohill 
On Tue, Oct 4, 2016 at 7:19 AM, wm4  wrote:

> On Mon,  3 Oct 2016 17:45:08 -0700
> Jon Toohill  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;
>
> How does mixing these even make sense?
>

mp3enc.c already uses initial_padding for the encoder delay, and as you
previously pointed out, mp3dec.c already uses AV_PKT_START_SKIP_SAMPLES for
the encoder delay. I'm not attempting to solve the inconsistency in this
patch set.


> >  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
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-10-04 Thread James Almer 
On 10/4/2016 12:52 PM, Hendrik Leppkes wrote:
> On Tue, Oct 4, 2016 at 2:45 AM, Jon Toohill
>  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

TESTgapless-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,

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

2016-10-04 Thread Michael Niedermayer 
On Mon, Oct 03, 2016 at 05:45:08PM -0700, Jon Toohill 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

isnt this exactly correct before and wrong after this change ?

zero signal before and zero signal after the original is what one would
expect and equal length and high correlation

every number that changes gets worse

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Never trust a computer, one day, it may think you are the virus. -- Compn


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-10-04 Thread wm4 
On Mon,  3 Oct 2016 17:45:08 -0700
Jon Toohill  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;

How does mixing these even make sense?

>  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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-10-03 Thread Jon Toohill 
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
-- 
2.8.0.rc3.226.g39d4020

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel