MediaRecorder: wire the bitRate settings in Blink and content (2nd go)

content/renderer/media/audio_track_recorder.cc

 // Copyright 2015 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 "content/renderer/media/audio_track_recorder.h"
 
 #include <stdint.h>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 27 matching lines @@
 }  // anonymous namespace
 
 // Nested class encapsulating opus-related encoding details.
 // AudioEncoder is created and destroyed on ATR's main thread (usually the
 // main render thread) but otherwise should operate entirely on
 // |encoder_thread_|, which is owned by AudioTrackRecorder. Be sure to delete
 // |encoder_thread_| before deleting the AudioEncoder using it.
 class AudioTrackRecorder::AudioEncoder
     : public base::RefCountedThreadSafe<AudioEncoder> {
  public:
-  explicit AudioEncoder(const OnEncodedAudioCB& on_encoded_audio_cb)
-      : on_encoded_audio_cb_(on_encoded_audio_cb), opus_encoder_(nullptr) {
-    // AudioEncoder is constructed on the thread that ATR lives on, but should
-    // operate only on the encoder thread after that. Reset
-    // |encoder_thread_checker_| here, as the next call to CalledOnValidThread()
-    // will be from the encoder thread.
-    encoder_thread_checker_.DetachFromThread();
-  }
+  AudioEncoder(const OnEncodedAudioCB& on_encoded_audio_cb,
+               int32_t bits_per_second);
 
   void OnSetFormat(const media::AudioParameters& params);
 
   void EncodeAudio(scoped_ptr<media::AudioBus> audio_bus,
                    const base::TimeTicks& capture_time);
 
  private:
   friend class base::RefCountedThreadSafe<AudioEncoder>;
 
   ~AudioEncoder();
 
   bool is_initialized() const { return !!opus_encoder_; }
 
   void DestroyExistingOpusEncoder();
 
   void TransferSamplesIntoBuffer(const media::AudioBus* audio_bus,
                                  int source_offset,
                                  int buffer_fill_offset,
                                  int num_samples);
   bool EncodeFromFilledBuffer(std::string* out);
 
   const OnEncodedAudioCB on_encoded_audio_cb_;
 
+  // Target bitrate for Opus. If 0, Opus provide automatic bitrate is used.
+  const int32_t bits_per_second_;
+
   base::ThreadChecker encoder_thread_checker_;
 
   // In the case where a call to EncodeAudio() cannot completely fill the
   // buffer, this points to the position at which to populate data in a later
   // call.
   int buffer_fill_end_;
 
   int frames_per_buffer_;
 
   // The duration of one set of frames of encoded audio samples.
   base::TimeDelta buffer_duration_;
 
   media::AudioParameters audio_params_;
 
   // Buffer for passing AudioBus data to OpusEncoder.
   scoped_ptr<float[]> buffer_;
 
   OpusEncoder* opus_encoder_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioEncoder);
 };
 
+AudioTrackRecorder::AudioEncoder::AudioEncoder(
+    const OnEncodedAudioCB& on_encoded_audio_cb,
+    int32_t bits_per_second)
+    : on_encoded_audio_cb_(on_encoded_audio_cb),
+      bits_per_second_(bits_per_second),
+      opus_encoder_(nullptr) {
+  // AudioEncoder is constructed on the thread that ATR lives on, but should
+  // operate only on the encoder thread after that. Reset
+  // |encoder_thread_checker_| here, as the next call to CalledOnValidThread()
+  // will be from the encoder thread.
+  encoder_thread_checker_.DetachFromThread();
+}
+
 AudioTrackRecorder::AudioEncoder::~AudioEncoder() {
   // We don't DCHECK that we're on the encoder thread here, as it should have
   // already been deleted at this point.
   DestroyExistingOpusEncoder();
 }
 
 void AudioTrackRecorder::AudioEncoder::OnSetFormat(
     const media::AudioParameters& params) {
   DCHECK(encoder_thread_checker_.CalledOnValidThread());
   if (audio_params_.Equals(params))
     return;
 
   DestroyExistingOpusEncoder();
 
-  if (!params.IsValid()) {
+  if (!params.IsValid() || params.channels() > 2) {
     DLOG(ERROR) << "Invalid audio params: " << params.AsHumanReadableString();
     return;
   }
 
   buffer_duration_ = base::TimeDelta::FromMilliseconds(
       AudioTrackRecorder::GetOpusBufferDuration(params.sample_rate()));
   if (buffer_duration_ == base::TimeDelta()) {
     DLOG(ERROR) << "Could not find a valid |buffer_duration| for the given "
                 << "sample rate: " << params.sample_rate();
     return;
   }
 
   frames_per_buffer_ =
       params.sample_rate() * buffer_duration_.InMilliseconds() / 1000;
   if (frames_per_buffer_ * params.channels() > MAX_SAMPLES_PER_BUFFER) {
     DLOG(ERROR) << "Invalid |frames_per_buffer_|: " << frames_per_buffer_;
     return;
   }
 
   // Initialize AudioBus buffer for OpusEncoder.
   buffer_fill_end_ = 0;
   buffer_.reset(new float[params.channels() * frames_per_buffer_]);
 
   // Initialize OpusEncoder.
+  DCHECK((params.sample_rate() != 48000) || (params.sample_rate() != 24000) ||

[brucedawson, 2016/01/26 18:28:47]

This DCHECK is accidentally tautological. There are no values of sample_rate()
that could cause it to trigger.

The "!=" operators should all be changed to "==".

This was found by the VC++ /analyze builder which reported:
audio_track_recorder.cc(150) : warning C6289: Incorrect operator:  mutual
exclusion over || is always a non-zero constant.  Did you intend to use &&
instead?

Note that its suggested fix is incorrect, but it is correct that there is a bug.

Note also that the /analyze builder does *not* automatically report these
warnings due to the very high false positive rate. I look at the new warnings a
few times a week and forward them. So, there is no fyi bot whose output you
missed.

+         (params.sample_rate() != 16000) || (params.sample_rate() != 12000) ||
+         (params.sample_rate() != 8000))
+      << "Opus supports only sample rates of {48, 24, 16, 12, 8}000, requested "
+      << params.sample_rate();
   int opus_result;
   opus_encoder_ = opus_encoder_create(params.sample_rate(), params.channels(),
                                       OPUS_APPLICATION_AUDIO, &opus_result);
   if (opus_result < 0) {
     DLOG(ERROR) << "Couldn't init opus encoder: " << opus_strerror(opus_result)
                 << ", sample rate: " << params.sample_rate()
                 << ", channels: " << params.channels();
     return;
   }
 
   // Note: As of 2013-10-31, the encoder in "auto bitrate" mode would use a
   // variable bitrate up to 102kbps for 2-channel, 48 kHz audio and a 10 ms
   // buffer duration. The opus library authors may, of course, adjust this in
   // later versions.
-  if (opus_encoder_ctl(opus_encoder_, OPUS_SET_BITRATE(OPUS_AUTO)) != OPUS_OK) {
-    DLOG(ERROR) << "Failed to set opus bitrate.";
+  const opus_int32 bitrate =
+      (bits_per_second_ > 0) ? bits_per_second_ : OPUS_AUTO;
+  if (opus_encoder_ctl(opus_encoder_, OPUS_SET_BITRATE(bitrate)) != OPUS_OK) {
+    DLOG(ERROR) << "Failed to set opus bitrate: " << bitrate;
     return;
   }
 
   audio_params_ = params;
 }
 
 void AudioTrackRecorder::AudioEncoder::EncodeAudio(
     scoped_ptr<media::AudioBus> audio_bus,
     const base::TimeTicks& capture_time) {
   DCHECK(encoder_thread_checker_.CalledOnValidThread());
@@ skipping 77 matching lines @@
   // If |result| in {0,1}, do nothing; the documentation says that a return
   // value of zero or one means the packet does not need to be transmitted.
   // Otherwise, we have an error.
   DLOG_IF(ERROR, result < 0) << __FUNCTION__
                              << " failed: " << opus_strerror(result);
   return false;
 }
 
 AudioTrackRecorder::AudioTrackRecorder(
     const blink::WebMediaStreamTrack& track,
-    const OnEncodedAudioCB& on_encoded_audio_cb)
+    const OnEncodedAudioCB& on_encoded_audio_cb,
+    int32_t bits_per_second)
     : track_(track),
-      encoder_(new AudioEncoder(media::BindToCurrentLoop(on_encoded_audio_cb))),
+      encoder_(new AudioEncoder(media::BindToCurrentLoop(on_encoded_audio_cb),
+                                bits_per_second)),
       encoder_thread_("AudioEncoderThread") {
   DCHECK(main_render_thread_checker_.CalledOnValidThread());
   DCHECK(!track_.isNull());
   DCHECK(track_.extraData());
 
   // Start the |encoder_thread_|. From this point on, |encoder_| should work
   // only on |encoder_thread_|, as enforced by DCHECKs.
   DCHECK(!encoder_thread_.IsRunning());
   encoder_thread_.Start();
 
@@ skipping 43 matching lines @@
     if (sample_rate * possible_duration % 1000 == 0) {
       return possible_duration;
     }
   }
 
   // Otherwise, couldn't find a good duration.
   return 0;
 }
 
 }  // namespace content