From ef69090a1a45be87e338ffddf4fc67cea2946e9f Mon Sep 17 00:00:00 2001 From: Ruud van Asseldonk Date: Wed, 22 Feb 2017 23:09:40 +0100 Subject: [PATCH] Add guard for negative qlp shift The spec does allow this, but it does not happen in practice. As it would complicate shifting due to arithmetic shift by a negative amount not being supported, I'll emit an "unsupported" error instead. If anybody has a real-world FLAC file that fails on this, a specialized version that deals with this case can be written. For now it is not worth the effort. --- src/subframe.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/subframe.rs b/src/subframe.rs index 4b8adcf..fd19d64 100644 --- a/src/subframe.rs +++ b/src/subframe.rs @@ -480,6 +480,8 @@ fn predict_lpc(raw_coefficients: &[i16], qlp_shift: i16, buffer: &mut [i32]) -> Result<()> { + debug_assert!(qlp_shift >= 0, "Right-shift by negative value is not allowed."); + debug_assert!(qlp_shift < 64, "Cannot shift by more than integer width."); // The decoded residuals are 25 bits at most (assuming subset FLAC of at // most 24 bits per sample, but there is the delta encoding for channels). // The coefficients are 16 bits at most, so their product is 41 bits. In @@ -585,6 +587,17 @@ fn decode_lpc(input: &mut Bitstream, let qlp_shift_unsig = try!(input.read_leq_u16(5)); let qlp_shift = extend_sign_u16(qlp_shift_unsig, 5); + // The spec does allow the qlp shift to be negative, but in practice this + // does not happen. Fully supporting it would be a performance hit, as an + // arithmetic shift by a negative amount is invalid, so this would incur a + // branch. If a real-world file ever hits this case, then we should consider + // making two LPC predictors, one for positive, and one for negative qlp. + if qlp_shift < 0 { + let msg = "a negative quantized linear predictor coefficient shift is \ + not supported, please file a bug."; + return Err(Error::Unsupported(msg)) + } + // Finally, the coefficients themselves. The order is at most 32, so all // coefficients can be kept on the stack. Store them in reverse, because // that how they are used in prediction.