On Wed, 5 Oct 2016 10:42:13 -0700
Jon Toohill <jtoohill-at-google....@ffmpeg.org> wrote:
> On Wed, Oct 5, 2016 at 10:40 AM, Jon Toohill <jtooh...@google.com> wrote:
>
> > On Tue, Oct 4, 2016 at 7:19 AM, wm4 <nfx...@googlemail.com> wrote:
> >
> >> On Mon, 3 Oct 2016 17:45:08 -0700
> >> Jon Toohill <jtoohill-at-google....@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;
> >>
> >> 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
