On 03/26/2014 06:25 AM, Alessandro Ghedini 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.
> ---
> libavformat/replaygain.c | 35 +++++++++++++++--------------------
> libavutil/replaygain.h | 3 +--
> 2 files changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/libavformat/replaygain.c b/libavformat/replaygain.c
> index cf4dbf8..d4d3d1a 100644
> --- a/libavformat/replaygain.c
> +++ b/libavformat/replaygain.c
> @@ -64,34 +64,29 @@ static int32_t parse_gain(const char *gain)
>
> static uint32_t parse_peak(const uint8_t *peak)
> {
> - int64_t val = 0;
> - int64_t scale = 1;
> + char *fraction;
> + unsigned int scale = 10000;
> + uint32_t mb = 0;
> + unsigned int db;
>
> 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++;
> + db = strtol(peak, &fraction, 0);
> + if (*fraction++ == '.') {
> + while (av_isdigit(*fraction) && scale) {
> + mb += scale * (*fraction - '0');
> + scale /= 10;
> + fraction++;
> + }
> }
>
> - return av_rescale(val, UINT32_MAX, scale);
> + if (db > (UINT32_MAX - mb) / 100000)
> + return 0;
> +
> + return db * 100000 + mb;
> }
>
This looks pretty darn close to parse_gain(), just without the sign.
Maybe the two functions could be merged.
> static int replaygain_export(AVStream *st,
> diff --git a/libavutil/replaygain.h b/libavutil/replaygain.h
> index 8447703..00d2873 100644
> --- a/libavutil/replaygain.h
> +++ b/libavutil/replaygain.h
> @@ -34,8 +34,7 @@ typedef struct AVReplayGain {
> */
> int32_t track_gain;
> /**
> - * Peak track amplitude, with UINT32_MAX representing full scale. 0 when
> - * unknown.
> + * Peak track amplitude. 0 when unknown.
> */
> uint32_t track_peak;
> /**
This is ok in general, but please specify the scale in the
documentation. Also this needs a libavutil minor bump, plus an entry in
APIchanges noting the change.
Thanks,
Justin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
