decoder/mad: Fix decode of LAME peak value

6d91b5c7 ("fix double promotions") changed how LAME peak values are decoded, producing large incorrect values that cause some MP3 files to play silently. Restore the original decode from MAD fixed-point format to double and document what it's doing. Fixes #1823

decoder/mad: Fix decode of LAME peak value
6ee3d010 · Simon Arlott · Max Kellermann · fc9626e2 · 6ee3d010 · 6ee3d010
Commit 6ee3d010 authored May 27, 2023 by Simon Arlott Committed by Max Kellermann Jun 02, 2023
Hide whitespace changes
Inline Side-by-side

Showing with 17 additions and 1 deletion

NEWS NEWS +2 -0

MadDecoderPlugin.cxx src/decoder/plugins/MadDecoderPlugin.cxx +15 -1

No files found.
--- a/NEWS
+++ b/NEWS
 ver 0.23.14 (not yet released)
+* decoder
+  - mad: fix calculation of LAME peak values

 ver 0.23.13 (2023/05/22)
 * input

--- a/src/decoder/plugins/MadDecoderPlugin.cxx
+++ b/src/decoder/plugins/MadDecoderPlugin.cxx
@@ -562,7 +562,21 @@ parse_lame(struct lame *lame, struct mad_bitptr *ptr, int *bitlen) noexcept

 	mad_bit_skip(ptr, 16);

-	lame->peak = MAD_F(mad_bit_read(ptr, 32) << 5); /* peak */
+	/* The lame peak value is a float multiplied by 2^23 and stored as an
+	 * unsigned integer (it is always positive). MAD's fixed-point format uses
+	 * 28 bits for the fractional part, so shift the 23 bit fraction up before
+	 * converting to a float.
+	 */
+	unsigned long peak_int = mad_bit_read(ptr, 32);
+
+#define LAME_PEAK_FRACBITS 23
+#if MAD_F_FRACBITS > LAME_PEAK_FRACBITS
+	peak_int <<= (MAD_F_FRACBITS - LAME_PEAK_FRACBITS);
+#elif LAME_PEAK_FRACBITS > MAD_F_FRACBITS
+	peak_int >>= (LAME_PEAK_FRACBITS - MAD_F_FRACBITS);
+#endif
+
+	lame->peak = mad_f_todouble(peak_int); /* peak */
 	FmtDebug(mad_domain, "LAME peak found: {}", lame->peak);

 	lame->track_gain = 0;