Speex encoding/decoding.

remoting/codec/audio_encoder_speex.cc

Index: remoting/codec/audio_encoder_speex.cc
diff --git a/remoting/codec/audio_encoder_speex.cc b/remoting/codec/audio_encoder_speex.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a1adcb3ecaf84680bf7425451bf4b7691f58acc1
--- /dev/null
+++ b/remoting/codec/audio_encoder_speex.cc
@@ -0,0 +1,127 @@
+// 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_encoder_speex.h"
+
+#include <string>
+#include <sstream>
+
+#include "base/basictypes.h"
+#include "base/logging.h"
+#include "base/stl_util.h"
+#include "remoting/proto/audio.pb.h"
+#include "third_party/speex/speex.h"
+#include "third_party/speex/include/speex/speex_stereo.h"
+
+namespace {
+const int kSpeexQuality = 8;

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

nit: Name this constant to indicate what quality we're requesting, e.g.
kSpeexHighQuality

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

Done.

+const int kEncodedDataBufferSize = 0xFF;
+}  // namespace
+
+namespace remoting {
+
+AudioEncoderSpeex::AudioEncoderSpeex()
+    : leftover_samples_(0) {
+  // Create and initialize the Speex structures.
+  speex_bits_.reset(new SpeexBits());
+  speex_bits_init(speex_bits_.get());
+  speex_state_ = speex_encoder_init(&speex_wb_mode);
+
+  // Set the encoding quality.
+  int quality = kSpeexQuality;
+  speex_encoder_ctl(speex_state_, SPEEX_SET_QUALITY, &quality);
+
+  // Get the frame size and construct the input buffer accordingly.
+  speex_encoder_ctl(speex_state_, SPEEX_GET_FRAME_SIZE, &speex_frame_size_);

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

Check that this operation succeeds? also see my comments in the decoder.

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

Done.

+  raw_data_buffer_.reset(
+      new int16[speex_frame_size_ * AudioPacket::CHANNELS_STEREO]);
+}
+
+AudioEncoderSpeex::~AudioEncoderSpeex() {
+  speex_encoder_destroy(speex_state_);
+  speex_bits_destroy(speex_bits_.get());
+}
+
+scoped_ptr<AudioPacket> AudioEncoderSpeex::Encode(
+    scoped_ptr<AudioPacket> packet) {
+  DCHECK_EQ(AudioPacket::ENCODING_RAW, packet->encoding());
+  DCHECK_EQ(1, packet->data_size());
+  DCHECK_EQ(AudioPacket::BYTES_PER_SAMPLE_2, packet->bytes_per_sample());
+  DCHECK_NE(AudioPacket::SAMPLING_RATE_INVALID, packet->sampling_rate());
+  DCHECK_EQ(AudioPacket::CHANNELS_STEREO, packet->channels());
+
+  // Put the samples onto the queue.
+  int samples_left = packet->data(0).size() / packet->bytes_per_sample();

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

It's better to divide this by number of channels() - you never want to have
samples for only some of the channels. I.e. this should be samples per channel,
not total number of samples.
It will be also more readable that way - you won't need to multiply by
channels() in many places in the code below.

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

Done.

+  const int16* next_sample =
+      reinterpret_cast<const int16*>(packet->data(0).data());
+
+  // Create a new packet of encoded data.
+  scoped_ptr<AudioPacket> encoded_packet(new AudioPacket());
+  encoded_packet->set_encoding(AudioPacket::ENCODING_SPEEX);
+  encoded_packet->set_sampling_rate(packet->sampling_rate());
+  encoded_packet->set_bytes_per_sample(packet->bytes_per_sample());
+  encoded_packet->set_channels(packet->channels());
+
+  while (leftover_samples_ + samples_left >=
+        speex_frame_size_ * packet->channels()) {

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

nit: indent one more space to align with (

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

Done.

+    if (leftover_samples_ > 0) {

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

Can you restructure this if...else... so that the SPEEX calls are outside it
entirely, rather than duplicated in the two blocks, and all it does is set up a
pointer & frame size to supply to those calls, either by setting up the data
buffer, or by passing the current packet's data directly?

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

Done.

+      int samples_needed =
+          speex_frame_size_ * packet->channels() - leftover_samples_;
+
+      memcpy(raw_data_buffer_.get() + leftover_samples_,
+             next_sample,
+             packet->bytes_per_sample() * samples_needed);
+
+      // Transform stereo to mono.
+      speex_encode_stereo_int(raw_data_buffer_.get(),
+                              speex_frame_size_,
+                              speex_bits_.get());

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

nit: Missing blank line after this?

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

Done.

+      // Encode the frame, treating all samples as integers.
+      speex_encode_int(speex_state_, raw_data_buffer_.get(), speex_bits_.get());
+
+      leftover_samples_ = 0;
+      next_sample += samples_needed;
+      samples_left -= samples_needed;
+    } else {
+      // Transform stereo to mono.
+      speex_encode_stereo_int(const_cast<int16*>(next_sample),
+                              speex_frame_size_,
+                              speex_bits_.get());

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

nit: blank line

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

Done.

+      // Encode the frame, treating all samples as integers.
+      speex_encode_int(speex_state_,
+                       const_cast<int16*>(next_sample),
+                       speex_bits_.get());

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

nit: and here

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

Done.

+      next_sample += speex_frame_size_ * packet->channels();
+      samples_left -= speex_frame_size_ * packet->channels();
+    }
+
+    std::string* new_data = encoded_packet->add_data();
+    new_data->resize(speex_bits_nbytes(speex_bits_.get()));
+
+    // Copy the encoded data from the bits structure into the buffer.
+    int bytes_written = speex_bits_write(speex_bits_.get(),
+                                         string_as_array(new_data),
+                                         new_data->size());
+
+    // Expect that the bytes are all written.
+    DCHECK_EQ(bytes_written, static_cast<int>(new_data->size()));
+
+    // Reset the bits structure for this frame.
+    speex_bits_reset(speex_bits_.get());
+  }
+
+  // Store the leftover samples.
+  if (samples_left > 0) {
+    CHECK_LE(leftover_samples_ + samples_left,
+             speex_frame_size_ * packet->channels());
+    memcpy(raw_data_buffer_.get() + leftover_samples_,
+           next_sample,
+           packet->bytes_per_sample() * samples_left);
+    leftover_samples_ += samples_left;
+  }
+
+  return encoded_packet.Pass();

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

Return null if there is no data in the packet (e.g. if the raw packet we've got
is smaller than the speex frame size).

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

Done.

+}
+
+}  // namespace remoting