Remove overhead from video bitrate.

webrtc/video/video_send_stream.cc

 /*
  *  Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 #include "webrtc/video/video_send_stream.h"
@@ skipping 31 matching lines @@
     RtcpBandwidthObserver* bandwidth_callback,
     TransportFeedbackObserver* transport_feedback_callback,
     RtcpRttStats* rtt_stats,
     RtpPacketSender* paced_sender,
     TransportSequenceNumberAllocator* transport_sequence_number_allocator,
     FlexfecSender* flexfec_sender,
     SendStatisticsProxy* stats_proxy,
     SendDelayStats* send_delay_stats,
     RtcEventLog* event_log,
     RateLimiter* retransmission_rate_limiter,
+    OverheadObserver* overhead_observer,
     size_t num_modules) {
   RTC_DCHECK_GT(num_modules, 0u);
   RtpRtcp::Configuration configuration;
   ReceiveStatistics* null_receive_statistics = configuration.receive_statistics;
   configuration.audio = false;
   configuration.receiver_only = false;
   configuration.flexfec_sender = flexfec_sender;
   configuration.receive_statistics = null_receive_statistics;
   configuration.outgoing_transport = outgoing_transport;
   configuration.intra_frame_callback = intra_frame_callback;
   configuration.bandwidth_callback = bandwidth_callback;
   configuration.transport_feedback_callback = transport_feedback_callback;
   configuration.rtt_stats = rtt_stats;
   configuration.rtcp_packet_type_counter_observer = stats_proxy;
   configuration.paced_sender = paced_sender;
   configuration.transport_sequence_number_allocator =
       transport_sequence_number_allocator;
   configuration.send_bitrate_observer = stats_proxy;
   configuration.send_frame_count_observer = stats_proxy;
   configuration.send_side_delay_observer = stats_proxy;
   configuration.send_packet_observer = send_delay_stats;
   configuration.event_log = event_log;
   configuration.retransmission_rate_limiter = retransmission_rate_limiter;
-
+  configuration.overhead_observer = overhead_observer;
   std::vector<RtpRtcp*> modules;
   for (size_t i = 0; i < num_modules; ++i) {
     RtpRtcp* rtp_rtcp = RtpRtcp::CreateRtpRtcp(configuration);
     rtp_rtcp->SetSendingStatus(false);
     rtp_rtcp->SetSendingMediaStatus(false);
     rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound);
     modules.push_back(rtp_rtcp);
   }
   return modules;
 }
@@ skipping 191 matching lines @@
 namespace internal {
 
 // VideoSendStreamImpl implements internal::VideoSendStream.
 // It is created and destroyed on |worker_queue|. The intent is to decrease the
 // need for locking and to ensure methods are called in sequence.
 // Public methods except |DeliverRtcp| must be called on |worker_queue|.
 // DeliverRtcp is called on the libjingle worker thread or a network thread.
 // An encoder may deliver frames through the EncodedImageCallback on an
 // arbitrary thread.
 class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver,
+                            public webrtc::OverheadObserver,
                             public webrtc::VCMProtectionCallback,
                             public ViEEncoder::EncoderSink {
  public:
   VideoSendStreamImpl(SendStatisticsProxy* stats_proxy,
                       rtc::TaskQueue* worker_queue,
                       CallStats* call_stats,
                       CongestionController* congestion_controller,
                       BitrateAllocator* bitrate_allocator,
                       SendDelayStats* send_delay_stats,
                       VieRemb* remb,
@@ skipping 33 matching lines @@
                             uint8_t fraction_loss,
                             int64_t rtt) override;
 
   // Implements webrtc::VCMProtectionCallback.
   int ProtectionRequest(const FecProtectionParams* delta_params,
                         const FecProtectionParams* key_params,
                         uint32_t* sent_video_rate_bps,
                         uint32_t* sent_nack_rate_bps,
                         uint32_t* sent_fec_rate_bps) override;
 
+  // Implements OverheadObserver.
+  void OnOverheadChanged(size_t overhead_bytes_per_packet) override;
+
   void OnEncoderConfigurationChanged(std::vector<VideoStream> streams,
                                      int min_transmit_bitrate_bps) override;
 
   // Implements EncodedImageCallback. The implementation routes encoded frames
   // to the |payload_router_| and |config.pre_encode_callback| if set.
   // Called on an arbitrary encoder callback thread.
   EncodedImageCallback::Result OnEncodedImage(
       const EncodedImage& encoded_image,
       const CodecSpecificInfo* codec_specific_info,
       const RTPFragmentationHeader* fragmentation) override;
@@ skipping 41 matching lines @@
   const std::vector<RtpRtcp*> rtp_rtcp_modules_;
   PayloadRouter payload_router_;
 
   // |weak_ptr_| to our self. This is used since we can not call
   // |weak_ptr_factory_.GetWeakPtr| from multiple sequences but it is ok to copy
   // an existing WeakPtr.
   rtc::WeakPtr<VideoSendStreamImpl> weak_ptr_;
   // |weak_ptr_factory_| must be declared last to make sure all WeakPtr's are
   // invalidated before any other members are destroyed.
   rtc::WeakPtrFactory<VideoSendStreamImpl> weak_ptr_factory_;
+
+  float payload_fraction_;
+  size_t transport_overhead_per_packet_;
 };
 
 // TODO(tommi): See if there's a more elegant way to create a task that creates
 // an object on the correct task queue.
 class VideoSendStream::ConstructionTask : public rtc::QueuedTask {
  public:
   ConstructionTask(std::unique_ptr<VideoSendStreamImpl>* send_stream,
                    rtc::Event* done_event,
                    SendStatisticsProxy* stats_proxy,
                    ViEEncoder* vie_encoder,
@@ skipping 316 matching lines @@
           bandwidth_observer_.get(),
           congestion_controller_->GetTransportFeedbackObserver(),
           call_stats_->rtcp_rtt_stats(),
           congestion_controller_->pacer(),
           congestion_controller_->packet_router(),
           flexfec_sender_.get(),
           stats_proxy_,
           send_delay_stats,
           event_log,
           congestion_controller_->GetRetransmissionRateLimiter(),
+          // TODO(michaelt): Use here "this" when BWE with overhead implemented.
+          nullptr,
           config_->rtp.ssrcs.size())),
       payload_router_(rtp_rtcp_modules_,
                       config_->encoder_settings.payload_type),
-      weak_ptr_factory_(this) {
+      weak_ptr_factory_(this),
+      payload_fraction_(1) {
   RTC_DCHECK_RUN_ON(worker_queue_);
   LOG(LS_INFO) << "VideoSendStreamInternal: " << config_->ToString();
   weak_ptr_ = weak_ptr_factory_.GetWeakPtr();
   module_process_thread_checker_.DetachFromThread();
 
   RTC_DCHECK(!config_->rtp.ssrcs.empty());
   RTC_DCHECK(call_stats_);
   RTC_DCHECK(congestion_controller_);
   RTC_DCHECK(remb_);
 
@@ skipping 16 matching lines @@
   }
 
   remb_->AddRembSender(rtp_rtcp_modules_[0]);
   rtp_rtcp_modules_[0]->SetREMBStatus(true);
 
   ConfigureProtection();
   ConfigureSsrcs();
 
   // TODO(pbos): Should we set CNAME on all RTP modules?
   rtp_rtcp_modules_.front()->SetCNAME(config_->rtp.c_name.c_str());
-  // 28 to match packet overhead in ModuleRtpRtcpImpl.
-  static const size_t kRtpPacketSizeOverhead = 28;
-  RTC_DCHECK_LE(config_->rtp.max_packet_size, 0xFFFFu + kRtpPacketSizeOverhead);

[stefan-webrtc, 2016/11/22 17:17:01]

Is there still a reasonable check we should make on the max_packet_size?

[michaelt, 2016/11/23 09:19:13]

We could check for lower than IP_MTU(1500)

-  const uint16_t mtu = static_cast<uint16_t>(config_->rtp.max_packet_size +
-                                             kRtpPacketSizeOverhead);
+
   for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
     rtp_rtcp->RegisterRtcpStatisticsCallback(stats_proxy_);
     rtp_rtcp->RegisterSendChannelRtpStatisticsCallback(stats_proxy_);
-    rtp_rtcp->SetMaxTransferUnit(mtu);
+    rtp_rtcp->SetMaxTransferUnit(config_->rtp.max_packet_size);
     rtp_rtcp->RegisterVideoSendPayload(
         config_->encoder_settings.payload_type,
         config_->encoder_settings.payload_name.c_str());
   }
 
   RTC_DCHECK(config_->encoder_settings.encoder);
   RTC_DCHECK_GE(config_->encoder_settings.payload_type, 0);
   RTC_DCHECK_LE(config_->encoder_settings.payload_type, 127);
 
   vie_encoder_->SetStartBitrate(bitrate_allocator_->GetStartBitrate(this));
@@ skipping 349 matching lines @@
                                                 : RtcpMode::kOff);
   }
 }
 
 uint32_t VideoSendStreamImpl::OnBitrateUpdated(uint32_t bitrate_bps,
                                                uint8_t fraction_loss,
                                                int64_t rtt) {
   RTC_DCHECK_RUN_ON(worker_queue_);
   RTC_DCHECK(payload_router_.active())
       << "VideoSendStream::Start has not been called.";
+
+  // Substract overhead from bitrate.
+  bitrate_bps *= payload_fraction_;

[minyue-webrtc, 2016/11/23 10:48:05]

I think we need to create a global flag to make clear transition from
no-overhead era to overhead era.

I think of "BANDWITDH_INCLUDE_OVERHEAD"

[stefan-webrtc, 2016/11/23 11:08:11]

I don't think that's enough. We have to only subtract overhead if we're using
send-side BWE. Call and CongestionController knows about this at least.

Once we have added a check like that I think we can enable this either under a
field trial, so that it's only used when audio frame length can change. This is
the more cautious approach. The other option is to enable it by default as it
doesn't really depend on varying audio frame lengths. This however requires that
all code related to overhead changes is landed, ideally in one CL.

You could land this behind a flag as suggested if you want to land it in
separate CLs, but then I'd suggest using a field trial anyway so that it's
easier to test it.

[michaelt, 2016/11/23 12:00:24]

Would prefer a field trial as well. I even think we may should test this without
without changing frame length. So that we would be able the to detect a bugs in
the "BWE with overhead" impl. 

+
   // Get the encoder target rate. It is the estimated network rate -
   // protection overhead.
   encoder_target_rate_bps_ = protection_bitrate_calculator_.SetTargetRates(
       bitrate_bps, stats_proxy_->GetSendFrameRate(), fraction_loss, rtt);
   uint32_t protection_bitrate = bitrate_bps - encoder_target_rate_bps_;
 
   encoder_target_rate_bps_ =
       std::min(encoder_max_bitrate_bps_, encoder_target_rate_bps_);
   vie_encoder_->OnBitrateUpdated(encoder_target_rate_bps_, fraction_loss, rtt);
   stats_proxy_->OnSetEncoderTargetRate(encoder_target_rate_bps_);
@@ skipping 39 matching lines @@
     rtp_rtcp->SetFecParameters(*delta_params, *key_params);
     rtp_rtcp->BitrateSent(&not_used, &module_video_rate, &module_fec_rate,
                           &module_nack_rate);
     *sent_video_rate_bps += module_video_rate;
     *sent_nack_rate_bps += module_nack_rate;
     *sent_fec_rate_bps += module_fec_rate;
   }
   return 0;
 }
 
+void VideoSendStreamImpl::OnOverheadChanged(size_t overhead_bytes_per_packet) {
+  RTC_DCHECK_RUN_ON(worker_queue_);
+  payload_fraction_ =
+      1.0 - (static_cast<float>(overhead_bytes_per_packet) /
+             (config_->rtp.max_packet_size + transport_overhead_per_packet_));

[stefan-webrtc, 2016/11/22 17:17:01]

This is an approximation assuming only max sized packets, right? I think it
would be good to improve it a bit since the overhead is a bit larger for low
bitrates (say 50 kbps)... Maybe do something like this in OnBitrateUpdated():
framerate = stats_proxy_->GetSendFrameRate();
bits_per_frame = bitrate_bps / framerate
packets_per_second = ceil(bits_per_frame / (8 * config_->rtp.max_packet_size)) *
framerate
overhead_per_second = 8 * overhead_bytes_per_packet * packets_per_second

I probably made some mistake, so please double check :)

[michaelt, 2016/11/23 09:19:13]

Yes I assumed only max packets.
I think we can simplify you calculation a little.
Framerate seams obsolete to me.
I would do:
packets_per_second = ceil(bitrate_bps / (8 * (config_->rtp.max_packet_size +
transport_overhead_per_packet_)));
overhead_per_second = 8 * overhead_bytes_per_packet * packets_per_second;

The ceil should make the difference to my old approach.

[stefan-webrtc, 2016/11/23 10:01:07]

Yes, you are probably right, the ceil is what's important.

+}
+
 void VideoSendStreamImpl::SetTransportOverhead(
     int transport_overhead_per_packet) {
-  for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
+  transport_overhead_per_packet_ = transport_overhead_per_packet;
+  RTC_DCHECK_LE(config_->rtp.max_packet_size,
+                0xFFFFu + transport_overhead_per_packet);
+  const uint16_t mtu = static_cast<uint16_t>(config_->rtp.max_packet_size +
+                                             transport_overhead_per_packet);
+  for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
     rtp_rtcp->SetTransportOverhead(transport_overhead_per_packet);
+    rtp_rtcp->SetMaxTransferUnit(mtu);
+  }
 }
 
 }  // namespace internal
 }  // namespace webrtc