On gio, apr 03, 2014 at 11:09:28 +0200, Alessandro Ghedini wrote:
> On Thu, Apr 03, 2014 at 09:18:20AM +0200, Anton Khirnov wrote:
> >
> > On Mon, 31 Mar 2014 12:48:57 +0200, Alessandro Ghedini
> > <[email protected]> wrote:
> > > According to the ReplayGain spec, the peak amplitude may overflow and may
> > > result
> > > in peak amplitude values greater than 1.0 with psychoacoustically coded
> > > audio,
> > > such as MP3. Fully compliant decoders must allow peak overflows.
> > >
> > > Additionally, having peak values in the 0<->UINT32_MAX scale makes it more
> > > difficult for applications to actually use the peak values (e.g. when
> > > implementing clipping prevention) since values have to be rescaled down.
> > >
> > > This patch corrects the peak parsing by removing the rescaling of the
> > > decoded
> > > values between 0 and UINT32_MAX and the 1.0 upper limit.
> > > ---
> > > Merged parse_gain() and parse_peak() into parse_value(), lavu minor
> > > version
> > > bump, APIchanges update, better peak documentation in replaygain.h
> > >
> > > doc/APIchanges | 4 ++++
> > > libavformat/replaygain.c | 56
> > > +++++++++++-------------------------------------
> > > libavutil/replaygain.h | 4 ++--
> > > libavutil/version.h | 2 +-
> > > 4 files changed, 20 insertions(+), 46 deletions(-)
> > >
> > > diff --git a/doc/APIchanges b/doc/APIchanges
> > > index d800253..e999c7e 100644
> > > --- a/doc/APIchanges
> > > +++ b/doc/APIchanges
> > > @@ -13,6 +13,10 @@ libavutil: 2013-12-xx
> > >
> > > API changes, most recent first:
> > >
> > > +2014-03-xx - xxxxxxx - lavu 53.09.0 - replaygain.h
> > > + Full scale for peak values is now 100000 (instead of UINT32_MAX) and
> > > values
> > > + may overflow.
> > > +
> > > 2014-02-xx - xxxxxxx - lavu 53.08.0 - frame.h
> > > Add av_frame_remove_side_data() for removing a single side data
> > > instance from a frame.
> > > diff --git a/libavformat/replaygain.c b/libavformat/replaygain.c
> > > index cf4dbf8..5e4bccd 100644
> > > --- a/libavformat/replaygain.c
> > > +++ b/libavformat/replaygain.c
> > > @@ -35,19 +35,19 @@
> > > #include "avformat.h"
> > > #include "replaygain.h"
> > >
> > > -static int32_t parse_gain(const char *gain)
> > > +static int32_t parse_value(const char *value, int32_t min)
> > > {
> > > char *fraction;
> > > int scale = 10000;
> > > int32_t mb = 0;
> > > int db;
> > >
> > > - if (!gain)
> > > - return INT32_MIN;
> > > + if (!value)
> > > + return min;
> > >
> > > - gain += strspn(gain, " \t");
> > > + value += strspn(value, " \t");
> > >
> > > - db = strtol(gain, &fraction, 0);
> > > + db = strtol(value, &fraction, 0);
> > > if (*fraction++ == '.') {
> > > while (av_isdigit(*fraction) && scale) {
> > > mb += scale * (*fraction - '0');
> > > @@ -57,41 +57,11 @@ static int32_t parse_gain(const char *gain)
> > > }
> > >
> > > if (abs(db) > (INT32_MAX - mb) / 100000)
> > > - return INT32_MIN;
> > > + return min;
> > >
> > > - return db * 100000 + FFSIGN(db) * mb;
> > > -}
> > > -
> > > -static uint32_t parse_peak(const uint8_t *peak)
> > > -{
> > > - int64_t val = 0;
> > > - int64_t scale = 1;
> > > -
> > > - if (!peak)
> > > - return 0;
> > > -
> > > - peak += strspn(peak, " \t");
> > > -
> > > - if (peak[0] == '1' && peak[1] == '.')
> > > - return UINT32_MAX;
> > > - else if (!(peak[0] == '0' && peak[1] == '.'))
> > > - return 0;
> > > -
> > > - peak += 2;
> > > -
> > > - while (av_isdigit(*peak)) {
> > > - int digit = *peak - '0';
> > > -
> > > - if (scale > INT64_MAX / 10)
> > > - break;
> > > -
> > > - val = 10 * val + digit;
> > > - scale *= 10;
> > > -
> > > - peak++;
> > > - }
> > > -
> > > - return av_rescale(val, UINT32_MAX, scale);
> > > + return (min == 0) ?
> > > + db * 100000 + mb :
> > > + db * 100000 + FFSIGN(db) * mb;
> >
> > Why the check here?
> > It will only make a difference if db is negative, in which case the result
> > will
> > be borked anyway.
>
> FFSIGN is defined as:
>
> #define FFSIGN(a) ((a) > 0 ? 1 : -1)
>
> so if db == 0 the result is negative.
>
> In fact, now that I think about this, the gain decoding is broken, since
> values
> that start with "0." are always going to be decoded as negative.
Not to mention that there's no such thing as -0, so strtol() always returns
non-negative 0 (which is then interpreted by FFSIGN() as negative).
What about using FFSIGN(strtod(value, ...)) to get the sign? (Not sure why
you didn't use strtod() in the first place, instead of doing the decoding
manually though).
Cheers
--
perl -E '$_=q;$/= @{[@_]};and s;\S+;<inidehG ordnasselA>;eg;say~~reverse'
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
