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

[Jon Toohill]

On Tue, Oct 4, 2016 at 9:10 AM, Michael Niedermayer <michael at niedermayer.cc>
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 at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>