From 323b132f5eabe8d459e66150eab9d8143cd12138 Mon Sep 17 00:00:00 2001 From: Minyue Date: Mon, 25 May 2015 13:49:37 +0200 Subject: [PATCH] Protect ACM decoder buffer in stereo. In https://code.google.com/p/webrtc/source/detail?r=8730, I did a protection on ACM decoder buffer from being overflow. However, the I misunderstood the return unit for PacketDuration(), and therefore, stereo decoders are not well protected. This CL fixed this. BUG=4361 R=henrik.lundin@webrtc.org Review URL: https://webrtc-codereview.appspot.com/47289004 Cr-Commit-Position: refs/heads/master@{#9275} --- .../audio_coding/codecs/audio_decoder.cc | 6 +++-- .../audio_coding/codecs/audio_decoder.h | 16 ++++++------ .../codecs/opus/interface/opus_interface.h | 5 ++-- .../audio_coding/neteq/neteq_impl_unittest.cc | 25 ++++++++++--------- 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/webrtc/modules/audio_coding/codecs/audio_decoder.cc b/webrtc/modules/audio_coding/codecs/audio_decoder.cc index 1ab2a7fec..9e88f897a 100644 --- a/webrtc/modules/audio_coding/codecs/audio_decoder.cc +++ b/webrtc/modules/audio_coding/codecs/audio_decoder.cc @@ -20,7 +20,8 @@ int AudioDecoder::Decode(const uint8_t* encoded, size_t encoded_len, int sample_rate_hz, size_t max_decoded_bytes, int16_t* decoded, SpeechType* speech_type) { int duration = PacketDuration(encoded, encoded_len); - if (duration >= 0 && duration * sizeof(int16_t) > max_decoded_bytes) { + if (duration >= 0 && + duration * Channels() * sizeof(int16_t) > max_decoded_bytes) { return -1; } return DecodeInternal(encoded, encoded_len, sample_rate_hz, decoded, @@ -31,7 +32,8 @@ int AudioDecoder::DecodeRedundant(const uint8_t* encoded, size_t encoded_len, int sample_rate_hz, size_t max_decoded_bytes, int16_t* decoded, SpeechType* speech_type) { int duration = PacketDurationRedundant(encoded, encoded_len); - if (duration >= 0 && duration * sizeof(int16_t) > max_decoded_bytes) { + if (duration >= 0 && + duration * Channels() * sizeof(int16_t) > max_decoded_bytes) { return -1; } return DecodeRedundantInternal(encoded, encoded_len, sample_rate_hz, decoded, diff --git a/webrtc/modules/audio_coding/codecs/audio_decoder.h b/webrtc/modules/audio_coding/codecs/audio_decoder.h index 1ac02c50f..8947e8116 100644 --- a/webrtc/modules/audio_coding/codecs/audio_decoder.h +++ b/webrtc/modules/audio_coding/codecs/audio_decoder.h @@ -36,8 +36,8 @@ class AudioDecoder { // Decodes |encode_len| bytes from |encoded| and writes the result in // |decoded|. The maximum bytes allowed to be written into |decoded| is - // |max_decoded_bytes|. The number of samples from all channels produced is - // in the return value. If the decoder produced comfort noise, |speech_type| + // |max_decoded_bytes|. Returns the total number of samples across all + // channels. If the decoder produced comfort noise, |speech_type| // is set to kComfortNoise, otherwise it is kSpeech. The desired output // sample rate is provided in |sample_rate_hz|, which must be valid for the // codec at hand. @@ -77,14 +77,14 @@ class AudioDecoder { // Returns the last error code from the decoder. virtual int ErrorCode(); - // Returns the duration in samples of the payload in |encoded| which is - // |encoded_len| bytes long. Returns kNotImplemented if no duration estimate - // is available, or -1 in case of an error. + // Returns the duration in samples-per-channel of the payload in |encoded| + // which is |encoded_len| bytes long. Returns kNotImplemented if no duration + // estimate is available, or -1 in case of an error. virtual int PacketDuration(const uint8_t* encoded, size_t encoded_len) const; - // Returns the duration in samples of the redandant payload in |encoded| which - // is |encoded_len| bytes long. Returns kNotImplemented if no duration - // estimate is available, or -1 in case of an error. + // Returns the duration in samples-per-channel of the redandant payload in + // |encoded| which is |encoded_len| bytes long. Returns kNotImplemented if no + // duration estimate is available, or -1 in case of an error. virtual int PacketDurationRedundant(const uint8_t* encoded, size_t encoded_len) const; diff --git a/webrtc/modules/audio_coding/codecs/opus/interface/opus_interface.h b/webrtc/modules/audio_coding/codecs/opus/interface/opus_interface.h index 27009a86a..dccc7ca71 100644 --- a/webrtc/modules/audio_coding/codecs/opus/interface/opus_interface.h +++ b/webrtc/modules/audio_coding/codecs/opus/interface/opus_interface.h @@ -288,7 +288,8 @@ int16_t WebRtcOpus_DecodeFec(OpusDecInst* inst, const uint8_t* encoded, * - payload : Encoded data pointer * - payload_length_bytes : Bytes of encoded data * - * Return value : The duration of the packet, in samples. + * Return value : The duration of the packet, in samples per + * channel. */ int WebRtcOpus_DurationEst(OpusDecInst* inst, const uint8_t* payload, @@ -308,7 +309,7 @@ int WebRtcOpus_DurationEst(OpusDecInst* inst, * - payload_length_bytes : Bytes of encoded data * * Return value : >0 - The duration of the FEC data in the - * packet in samples. + * packet in samples per channel. * 0 - No FEC data in the packet. */ int WebRtcOpus_FecDurationEst(const uint8_t* payload, diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc index 3823d9675..b03d1a290 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -805,16 +805,16 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) { UseNoMocks(); CreateInstance(); static const size_t kNetEqMaxFrameSize = 2880; // 60 ms @ 48 kHz. + static const int kChannels = 2; const uint8_t kPayloadType = 17; // Just an arbitrary number. const uint32_t kReceiveTime = 17; // Value doesn't matter for this test. const int kSampleRateHz = 8000; - const int kChannles = 1; const int kPayloadLengthSamples = 10 * kSampleRateHz / 1000; // 10 ms. const size_t kPayloadLengthBytes = 1; uint8_t payload[kPayloadLengthBytes]= {0}; - int16_t dummy_output[kPayloadLengthSamples] = {0}; + int16_t dummy_output[kPayloadLengthSamples * kChannels] = {0}; WebRtcRTPHeader rtp_header; rtp_header.header.payloadType = kPayloadType; rtp_header.header.sequenceNumber = 0x1234; @@ -829,7 +829,7 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) { MOCK_CONST_METHOD2(PacketDuration, int(const uint8_t*, size_t)); MOCK_METHOD5(DecodeInternal, int(const uint8_t*, size_t, int, int16_t*, SpeechType*)); - size_t Channels() const override { return 1; } + size_t Channels() const override { return kChannels; } } decoder_; const uint8_t kFirstPayloadValue = 1; @@ -838,7 +838,7 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) { EXPECT_CALL(decoder_, PacketDuration(Pointee(kFirstPayloadValue), kPayloadLengthBytes)) .Times(AtLeast(1)) - .WillRepeatedly(Return(kNetEqMaxFrameSize * kChannles + 1)); + .WillRepeatedly(Return(kNetEqMaxFrameSize + 1)); EXPECT_CALL(decoder_, DecodeInternal(Pointee(kFirstPayloadValue), _, _, _, _)) @@ -849,14 +849,15 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) { kSampleRateHz, _, _)) .Times(1) .WillOnce(DoAll(SetArrayArgument<3>(dummy_output, - dummy_output + kPayloadLengthSamples), + dummy_output + + kPayloadLengthSamples * kChannels), SetArgPointee<4>(AudioDecoder::kSpeech), - Return(kPayloadLengthSamples))); + Return(kPayloadLengthSamples * kChannels))); EXPECT_CALL(decoder_, PacketDuration(Pointee(kSecondPayloadValue), kPayloadLengthBytes)) .Times(AtLeast(1)) - .WillRepeatedly(Return(kNetEqMaxFrameSize * kChannles)); + .WillRepeatedly(Return(kNetEqMaxFrameSize)); EXPECT_EQ(NetEq::kOK, neteq_->RegisterExternalDecoder( @@ -878,7 +879,7 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) { neteq_->InsertPacket( rtp_header, payload, kPayloadLengthBytes, kReceiveTime)); - const int kMaxOutputSize = 10 * kSampleRateHz / 1000; + const int kMaxOutputSize = 10 * kSampleRateHz / 1000 * kChannels; int16_t output[kMaxOutputSize]; int samples_per_channel; int num_channels; @@ -888,14 +889,14 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) { &samples_per_channel, &num_channels, &type)); EXPECT_EQ(NetEq::kOtherDecoderError, neteq_->LastError()); - EXPECT_EQ(kMaxOutputSize, samples_per_channel); - EXPECT_EQ(kChannles, num_channels); + EXPECT_EQ(kMaxOutputSize, samples_per_channel * kChannels); + EXPECT_EQ(kChannels, num_channels); EXPECT_EQ(NetEq::kOK, neteq_->GetAudio(kMaxOutputSize, output, &samples_per_channel, &num_channels, &type)); - EXPECT_EQ(kMaxOutputSize, samples_per_channel); - EXPECT_EQ(kChannles, num_channels); + EXPECT_EQ(kMaxOutputSize, samples_per_channel * kChannels); + EXPECT_EQ(kChannels, num_channels); } } // namespace webrtc