Update smoothed bitrate.

webrtc/audio/audio_send_stream.cc

Index: webrtc/audio/audio_send_stream.cc
diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc
index aff605fe6a6845b995fd2082d458d8e5b9f45f4c..5a5ca776683971c1e19ae83232083ad8d8b4ee01 100644
--- a/webrtc/audio/audio_send_stream.cc
+++ b/webrtc/audio/audio_send_stream.cc
@@ -11,6 +11,7 @@
 #include "webrtc/audio/audio_send_stream.h"
 
 #include <string>
+#include <utility>
 
 #include "webrtc/audio/audio_state.h"
 #include "webrtc/audio/conversion.h"
@@ -41,6 +42,23 @@ bool IsCodec(const webrtc::CodecInst& codec, const char* ref_name) {
 }  // namespace
 
 namespace internal {
+
+class AudioSendStream::AdaptCodecTask : public rtc::QueuedTask {
+ public:
+  explicit AdaptCodecTask(const rtc::WeakPtr<AudioSendStream>& send_stream)
+      : send_stream_(std::move(send_stream)) {}
+
+ private:
+  bool Run() override {
+    if (send_stream_) {

[stefan-webrtc, 2016/12/13 12:29:55]

Are we not able to DCHECK this because the worker queue lives longer than the
send stream?

[michaelt, 2016/12/13 17:01:26]

Yes and that is as well the reason i used weak pointer her.

+      send_stream_->AdaptCodec();
+    }
+    return true;
+  }
+
+  rtc::WeakPtr<AudioSendStream> send_stream_;
+};
+
 AudioSendStream::AudioSendStream(
     const webrtc::AudioSendStream::Config& config,
     const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
@@ -105,6 +123,8 @@ void AudioSendStream::Start() {
     RTC_DCHECK_GE(config_.max_bitrate_bps, config_.min_bitrate_bps);
     rtc::Event thread_sync_event(false /* manual_reset */, false);
     worker_queue_->PostTask([this, &thread_sync_event] {
+      weak_ptr_factory_.reset(new rtc::WeakPtrFactory<AudioSendStream>(this));
+      weak_ptr_ = weak_ptr_factory_->GetWeakPtr();
       bitrate_allocator_->AddObserver(this, config_.min_bitrate_bps,
                                       config_.max_bitrate_bps, 0, true);
       thread_sync_event.Set();
@@ -124,6 +144,7 @@ void AudioSendStream::Stop() {
   rtc::Event thread_sync_event(false /* manual_reset */, false);
   worker_queue_->PostTask([this, &thread_sync_event] {
     bitrate_allocator_->RemoveObserver(this);
+    weak_ptr_factory_.reset(nullptr);
     thread_sync_event.Set();
   });
   thread_sync_event.Wait(rtc::Event::kForever);
@@ -234,6 +255,7 @@ uint32_t AudioSendStream::OnBitrateUpdated(uint32_t bitrate_bps,
                                            uint8_t fraction_loss,
                                            int64_t rtt,
                                            int64_t probing_interval_ms) {
+  RTC_DCHECK_RUN_ON(worker_queue_);
   RTC_DCHECK_GE(bitrate_bps,
                 static_cast<uint32_t>(config_.min_bitrate_bps));
   // The bitrate allocator might allocate an higher than max configured bitrate
@@ -244,6 +266,10 @@ uint32_t AudioSendStream::OnBitrateUpdated(uint32_t bitrate_bps,
 
   channel_proxy_->SetBitrate(bitrate_bps, probing_interval_ms);
 
+  if (first_update_bitrate_()) {

[stefan-webrtc, 2016/12/13 12:29:55]

I think it would make sense to comment on why this is only done the first time.

[michaelt, 2016/12/13 17:01:26]

Done.

+    worker_queue_->PostTask([this]() { AdaptCodec(); });

[the sun, 2016/12/15 14:58:37]

When is OnBitrateUpdated() called?

IIUC you're starting the periodic task here and it will call
channel_proxy_->AdaptCodec(); the first time, and then not call it until Start()
has been called on the stream.

The conditions here appear to me unclear and subtle. I'd like to make the code
simpler.

[michaelt, 2016/12/19 14:43:28]

I removed the conditions here a start the task in Start();

+  }
+
   // The amount of audio protection is not exposed by the encoder, hence
   // always returning 0.
   return 0;
@@ -386,5 +412,14 @@ bool AudioSendStream::SetupSendCodec() {
   return true;
 }
 
+void AudioSendStream::AdaptCodec() {
+  RTC_DCHECK_RUN_ON(worker_queue_);
+  channel_proxy_->AdaptCodec();
+  constexpr uint32_t kAdaptCodecIntervalMs = 200;
+  worker_queue_->PostDelayedTask(

[stefan-webrtc, 2016/12/13 12:29:55]

Is there a point in posting another task here? Can't you do the work directly
since you're already on the worker queue?

[michaelt, 2016/12/13 17:01:26]

This post is the trick of this CL.
We have to call channel_proxy_->AdaptCodec() in timely base(each 200ms) as long
that AudioSendStream is running.

+      std::unique_ptr<rtc::QueuedTask>(new AdaptCodecTask(weak_ptr_)),

[the sun, 2016/12/13 16:43:24]

Can you just post a lambda here, calling AdaptCodec()?

I'm not sure you need to weak_ptr logic either - you're using it as a flag, so
why not just have a flag?

[michaelt, 2016/12/13 17:01:26]

The QueuedTask and the weak pointer implemented to make sure we will not run in
to trouble when AudioSendStream is deleted but we still have a AdaptCodecTask in
the queue. Because the working queue is not owned by the AudioSendStream the
queue can life longer than the stream.
     

[the sun, 2016/12/15 14:58:37]

I see. A subtle effect here is that you won't be receiving calls strictly
periodically. AudioSendStreams are recreated now and then, even while sending is
started. Just FYI.

I think you can arrange the code a little nicer by following example #3 in
base/task_queue.h - reposting "this". IIUC you could keep the WeakPtrFactory in
AudioSendStream and the WeakPtr only in AdaptCodecTask. I think that would be a
nicer separation.

[michaelt, 2016/12/19 14:43:28]

Done.

+      kAdaptCodecIntervalMs);
+}
+
 }  // namespace internal
 }  // namespace webrtc