Speex encoding/decoding.

remoting/codec/audio_decoder_speex.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "remoting/codec/audio_decoder_speex.h"
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/logging.h"
+#include "base/stl_util.h"
+#include "remoting/proto/audio.pb.h"
+
+extern "C" {
+#include "third_party/speex/speex.h"
+#include "third_party/speex/include/speex/speex_callbacks.h"
+#include "third_party/speex/include/speex/speex_stereo.h"
+}

[Sergey Ulanov, 2012/08/20 22:50:39]

nit:  // extern "C"

[kxing, 2012/08/21 16:56:11]

Done.

+
+namespace remoting {
+
+AudioDecoderSpeex::AudioDecoderSpeex() {
+  // Create and initialize the Speex structures.
+  speex_bits_.reset(new SpeexBits());
+  speex_bits_init(speex_bits_.get());
+  speex_state_ = speex_decoder_init(&speex_wb_mode);
+
+  // Create and initialize the Speex stereo state.
+  speex_stereo_state_ = speex_stereo_state_init();
+
+  // Create and initialize the stereo callback.
+  speex_callback_.reset(new SpeexCallback());
+  speex_callback_->callback_id = SPEEX_INBAND_STEREO;
+  speex_callback_->func = speex_std_stereo_request_handler;
+  speex_callback_->data = speex_stereo_state_;
+
+  int result;
+
+  // Turn perceptual enhancer.

[Wez, 2012/08/21 01:14:13]

nit: Turn on perceptual enhancer? Perhaps indicate in the comment what this is
actually for.

[kxing, 2012/08/21 16:56:11]

Done.

+  int enhancer = 1;
+  result = speex_decoder_ctl(speex_state_, SPEEX_SET_ENH, &enhancer);
+  if (result < 0) {
+    LOG(ERROR) << "Failed to turn on the perceptual enhancer";
+    return;

[Wez, 2012/08/21 01:14:13]

You're early exiting here, but you don't have any member initializers above, so
there is a risk that if one of these errors occurs you end up with a
partially-uninitialized object.  Also, how does the caller detect that the
decoder is broken in that case?

[kxing, 2012/08/21 16:56:11]

Done.

+  }
+
+  // Get the frame size.

[Wez, 2012/08/21 01:14:13]

nit: Similarly, indicate _why_ we need to get the frame size.

[kxing, 2012/08/21 16:56:11]

Done.

+  result = speex_decoder_ctl(speex_state_,
+                             SPEEX_GET_FRAME_SIZE,
+                             &speex_frame_size_);
+  if (result < 0) {
+    LOG(ERROR) << "Failed to get the frame size";

[Sergey Ulanov, 2012/08/20 22:50:39]

Should we also remember that we failed to initialize and drop everything in
Decode()? E.g. you can do it by setting speex_frame_size_ to some invalid value.
Otherwise we will be trying to decode using uninitialized value of
speex_frame_size_

Or alternatively you can replace this with LOG(FATAL) so we just crash if this
error ever happens (we don't expect it ever happen under normal conditions
anyway).

[kxing, 2012/08/21 16:56:11]

Done.

+    return;
+  }
+
+  // Set the stereo callback.

[Wez, 2012/08/21 01:14:13]

nit: Similarly, indicate what the significance of the stereo callback is; is
this just a stereo equivalent of a mono audio callback, for instance?

[kxing, 2012/08/21 16:56:11]

Done.

+  result = speex_decoder_ctl(speex_state_,
+                             SPEEX_SET_HANDLER,
+                             speex_callback_.get());
+  if (result < 0) {
+    LOG(ERROR) << "Failed to set the stereo callback.";
+    return;
+  }
+}
+
+AudioDecoderSpeex::~AudioDecoderSpeex() {
+  speex_stereo_state_destroy(speex_stereo_state_);
+  speex_decoder_destroy(speex_state_);
+  speex_bits_destroy(speex_bits_.get());
+}
+
+scoped_ptr<AudioPacket> AudioDecoderSpeex::Decode(
+    scoped_ptr<AudioPacket> packet) {
+  if ((packet->encoding() != AudioPacket::ENCODING_SPEEX) ||
+      (packet->bytes_per_sample() != AudioPacket::BYTES_PER_SAMPLE_2) ||
+      (packet->sampling_rate() == AudioPacket::SAMPLING_RATE_INVALID) ||
+      (packet->channels() != AudioPacket::CHANNELS_STEREO)) {
+    LOG(WARNING) << "Received a corrupted packet.";

[Sergey Ulanov, 2012/08/20 22:50:39]

It's not necessarily corrupt. It's just not supported (e.g. Mono).

[kxing, 2012/08/21 16:56:11]

Done.

+    return scoped_ptr<AudioPacket>(NULL);
+  }
+
+  // Create a new packet of decoded data.
+  scoped_ptr<AudioPacket> decoded_packet(new AudioPacket());
+  decoded_packet->set_encoding(AudioPacket::ENCODING_RAW);
+  decoded_packet->set_sampling_rate(packet->sampling_rate());
+  decoded_packet->set_bytes_per_sample(packet->bytes_per_sample());
+  decoded_packet->set_channels(packet->channels());
+
+  std::string* decoded_data = decoded_packet->add_data();
+  decoded_data->resize(packet->data_size() *
+                       speex_frame_size_ *
+                       packet->bytes_per_sample() *
+                       packet->channels());
+  int16* samples = reinterpret_cast<int16*>(string_as_array(decoded_data));
+
+  for (int i = 0; i < packet->data_size(); ++i) {
+    // Read the bytes into the bits structure.
+    speex_bits_read_from(speex_bits_.get(),
+                         string_as_array(packet->mutable_data(i)),
+                         packet->data(i).size());
+
+    // Decode the frame and store it in the buffer.
+    int status = speex_decode_int(speex_state_, speex_bits_.get(), samples);
+    if (status < 0) {
+      LOG(ERROR) << "Error in decoding Speex data.";
+      return scoped_ptr<AudioPacket>(NULL);
+    }
+    // Transform mono to stereo.
+    speex_decode_stereo_int(samples, speex_frame_size_, speex_stereo_state_);
+
+    samples += (speex_frame_size_ * packet->channels());
+  }
+
+  return decoded_packet.Pass();
+}
+
+}  // namespace remoting