Reland "Move allocation and rtp conversion logic out of payload router."

This reverts commit c2406e4. Reason for revert: Reland by removing the conflict with the broken CL. Original change's description: > Revert "Move allocation and rtp conversion logic out of payload router." > > This reverts commit 1da4d79. > > Reason for revert: Need to revert https://webrtc-review.googlesource.com/c/src/+/88220 > > This causes a merge conflict. So need to revert this first. > > Original change's description: > > Move allocation and rtp conversion logic out of payload router. > > > > Makes it easier to write tests, and allows for moving rtp module > > ownership into the payload router in the future. > > > > The RtpPayloadParams class is split into declaration and definition and > > moved into separate files. > > > > Bug: webrtc:9517 > > Change-Id: I8700628edff19abcacfe8d3a20e4ba7476f712ad > > Reviewed-on: https://webrtc-review.googlesource.com/88564 > > Commit-Queue: Stefan Holmer <[email protected]> > > Reviewed-by: Sebastian Jansson <[email protected]> > > Cr-Commit-Position: refs/heads/master@{#23983} > > [email protected],[email protected],[email protected] > > Change-Id: I342c4bf483d975c87c706fe7f76f44e2dc60fe4c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: webrtc:9517 > Reviewed-on: https://webrtc-review.googlesource.com/88821 > Reviewed-by: JT Teh <[email protected]> > Commit-Queue: JT Teh <[email protected]> > Cr-Commit-Position: refs/heads/master@{#23991} [email protected],[email protected],[email protected],[email protected],[email protected],[email protected] Change-Id: I154145cdbc668feee86dbe78860147a6954fee6c No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:9517 Reviewed-on: https://webrtc-review.googlesource.com/89020 Commit-Queue: Stefan Holmer <[email protected]> Reviewed-by: Stefan Holmer <[email protected]> Cr-Commit-Position: refs/heads/master@{#23996}
jiashilei-1985 · Jul 17, 2018 · f704468 · f704468
1 parent e250645
commit f704468
Show file tree

Hide file tree

Showing 12 changed files with 623 additions and 637 deletions.
diff --git a/BUILD.gn b/BUILD.gn
@@ -422,6 +422,7 @@ if (rtc_include_tests) {
       "api:rtc_api_unittests",
       "api/audio/test:audio_api_unittests",
       "api/audio_codecs/test:audio_codecs_api_unittests",
+      "api/video/test:rtc_api_video_unittests",
       "api/video_codecs/test:video_codecs_api_unittests",
       "p2p:libstunprober_unittests",
       "p2p:rtc_p2p_unittests",

diff --git a/api/video/test/BUILD.gn b/api/video/test/BUILD.gn
@@ -0,0 +1,27 @@
+# Copyright (c) 2018 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.
+
+import("../../../webrtc.gni")
+
+rtc_source_set("rtc_api_video_unittests") {
+  testonly = true
+
+  sources = [
+    "video_bitrate_allocation_unittest.cc",
+  ]
+
+  if (!build_with_chromium && is_clang) {
+    # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163).
+    suppressed_configs += [ "//build/config/clang:find_bad_constructs" ]
+  }
+
+  deps = [
+    "..:video_bitrate_allocation",
+    "../../../test:test_support",
+  ]
+}
diff --git a/api/video/test/video_bitrate_allocation_unittest.cc b/api/video/test/video_bitrate_allocation_unittest.cc
@@ -0,0 +1,63 @@
+/*
+ *  Copyright (c) 2018 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 <memory>
+#include <string>
+
+#include "api/video/video_bitrate_allocation.h"
+#include "test/gtest.h"
+
+namespace webrtc {
+TEST(VideoBitrateAllocation, SimulcastTargetBitrate) {
+  VideoBitrateAllocation bitrate;
+  bitrate.SetBitrate(0, 0, 10000);
+  bitrate.SetBitrate(0, 1, 20000);
+  bitrate.SetBitrate(1, 0, 40000);
+  bitrate.SetBitrate(1, 1, 80000);
+
+  VideoBitrateAllocation layer0_bitrate;
+  layer0_bitrate.SetBitrate(0, 0, 10000);
+  layer0_bitrate.SetBitrate(0, 1, 20000);
+
+  VideoBitrateAllocation layer1_bitrate;
+  layer1_bitrate.SetBitrate(0, 0, 40000);
+  layer1_bitrate.SetBitrate(0, 1, 80000);
+
+  std::vector<absl::optional<VideoBitrateAllocation>> layer_allocations =
+      bitrate.GetSimulcastAllocations();
+
+  EXPECT_EQ(layer0_bitrate, layer_allocations[0]);
+  EXPECT_EQ(layer1_bitrate, layer_allocations[1]);
+}
+
+TEST(VideoBitrateAllocation, SimulcastTargetBitrateWithInactiveStream) {
+  // Create bitrate allocation with bitrate only for the first and third stream.
+  VideoBitrateAllocation bitrate;
+  bitrate.SetBitrate(0, 0, 10000);
+  bitrate.SetBitrate(0, 1, 20000);
+  bitrate.SetBitrate(2, 0, 40000);
+  bitrate.SetBitrate(2, 1, 80000);
+
+  VideoBitrateAllocation layer0_bitrate;
+  layer0_bitrate.SetBitrate(0, 0, 10000);
+  layer0_bitrate.SetBitrate(0, 1, 20000);
+
+  VideoBitrateAllocation layer2_bitrate;
+  layer2_bitrate.SetBitrate(0, 0, 40000);
+  layer2_bitrate.SetBitrate(0, 1, 80000);
+
+  std::vector<absl::optional<VideoBitrateAllocation>> layer_allocations =
+      bitrate.GetSimulcastAllocations();
+
+  EXPECT_EQ(layer0_bitrate, layer_allocations[0]);
+  EXPECT_FALSE(layer_allocations[1]);
+  EXPECT_EQ(layer2_bitrate, layer_allocations[2]);
+}
+}  // namespace webrtc
diff --git a/api/video/video_bitrate_allocation.cc b/api/video/video_bitrate_allocation.cc
@@ -107,6 +107,23 @@ std::vector<uint32_t> VideoBitrateAllocation::GetTemporalLayerAllocation(
   return temporal_rates;
 }
 
+std::vector<absl::optional<VideoBitrateAllocation>>
+VideoBitrateAllocation::GetSimulcastAllocations() const {
+  std::vector<absl::optional<VideoBitrateAllocation>> bitrates;
+  for (size_t si = 0; si < kMaxSpatialLayers; ++si) {
+    absl::optional<VideoBitrateAllocation> layer_bitrate;
+    if (IsSpatialLayerUsed(si)) {
+      layer_bitrate = VideoBitrateAllocation();
+      for (int tl = 0; tl < kMaxTemporalStreams; ++tl) {
+        if (HasBitrate(si, tl))
+          layer_bitrate->SetBitrate(0, tl, GetBitrate(si, tl));
+      }
+    }
+    bitrates.push_back(layer_bitrate);
+  }
+  return bitrates;
+}
+
 bool VideoBitrateAllocation::operator==(
     const VideoBitrateAllocation& other) const {
   for (size_t si = 0; si < kMaxSpatialLayers; ++si) {

diff --git a/api/video/video_bitrate_allocation.h b/api/video/video_bitrate_allocation.h
@@ -62,6 +62,12 @@ class VideoBitrateAllocation {
   // layer with a defined bitrate.
   std::vector<uint32_t> GetTemporalLayerAllocation(size_t spatial_index) const;
 
+  // Returns one VideoBitrateAllocation for each spatial layer. This is used to
+  // configure simulcast streams. Note that the length of the returned vector is
+  // always kMaxSpatialLayers, the optional is unset for unused layers.
+  std::vector<absl::optional<VideoBitrateAllocation>> GetSimulcastAllocations()
+      const;
+
   uint32_t get_sum_bps() const { return sum_; }  // Sum of all bitrates.
   uint32_t get_sum_kbps() const {
     // Round down to not exceed the allocated bitrate.

diff --git a/call/BUILD.gn b/call/BUILD.gn
@@ -100,6 +100,8 @@ rtc_source_set("rtp_sender") {
   sources = [
     "payload_router.cc",
     "payload_router.h",
+    "rtp_payload_params.cc",
+    "rtp_payload_params.h",
     "rtp_transport_controller_send.cc",
     "rtp_transport_controller_send.h",
   ]
@@ -281,6 +283,7 @@ if (rtc_include_tests) {
       "rtcp_demuxer_unittest.cc",
       "rtp_bitrate_configurator_unittest.cc",
       "rtp_demuxer_unittest.cc",
+      "rtp_payload_params_unittest.cc",
       "rtp_rtcp_demuxer_helper_unittest.cc",
       "rtx_receive_stream_unittest.cc",
     ]

diff --git a/call/payload_router.cc b/call/payload_router.cc
@@ -14,155 +14,27 @@
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/video_coding/include/video_codec_interface.h"
 #include "rtc_base/checks.h"
-#include "rtc_base/random.h"
-#include "rtc_base/timeutils.h"
 
 namespace webrtc {
 
 namespace {
-// Map information from info into rtp.
-void CopyCodecSpecific(const CodecSpecificInfo* info, RTPVideoHeader* rtp) {
-  RTC_DCHECK(info);
-  rtp->codec = info->codecType;
+absl::optional<size_t> GetSimulcastIdx(const CodecSpecificInfo* info) {
+  if (!info)
+    return absl::nullopt;
   switch (info->codecType) {
-    case kVideoCodecVP8: {
-      rtp->vp8().InitRTPVideoHeaderVP8();
-      rtp->vp8().nonReference = info->codecSpecific.VP8.nonReference;
-      rtp->vp8().temporalIdx = info->codecSpecific.VP8.temporalIdx;
-      rtp->vp8().layerSync = info->codecSpecific.VP8.layerSync;
-      rtp->vp8().keyIdx = info->codecSpecific.VP8.keyIdx;
-      rtp->simulcastIdx = info->codecSpecific.VP8.simulcastIdx;
-      return;
-    }
-    case kVideoCodecVP9: {
-      rtp->vp9().InitRTPVideoHeaderVP9();
-      rtp->vp9().inter_pic_predicted =
-          info->codecSpecific.VP9.inter_pic_predicted;
-      rtp->vp9().flexible_mode = info->codecSpecific.VP9.flexible_mode;
-      rtp->vp9().ss_data_available = info->codecSpecific.VP9.ss_data_available;
-      rtp->vp9().non_ref_for_inter_layer_pred =
-          info->codecSpecific.VP9.non_ref_for_inter_layer_pred;
-      rtp->vp9().temporal_idx = info->codecSpecific.VP9.temporal_idx;
-      rtp->vp9().spatial_idx = info->codecSpecific.VP9.spatial_idx;
-      rtp->vp9().temporal_up_switch =
-          info->codecSpecific.VP9.temporal_up_switch;
-      rtp->vp9().inter_layer_predicted =
-          info->codecSpecific.VP9.inter_layer_predicted;
-      rtp->vp9().gof_idx = info->codecSpecific.VP9.gof_idx;
-      rtp->vp9().num_spatial_layers =
-          info->codecSpecific.VP9.num_spatial_layers;
-
-      if (info->codecSpecific.VP9.ss_data_available) {
-        rtp->vp9().spatial_layer_resolution_present =
-            info->codecSpecific.VP9.spatial_layer_resolution_present;
-        if (info->codecSpecific.VP9.spatial_layer_resolution_present) {
-          for (size_t i = 0; i < info->codecSpecific.VP9.num_spatial_layers;
-               ++i) {
-            rtp->vp9().width[i] = info->codecSpecific.VP9.width[i];
-            rtp->vp9().height[i] = info->codecSpecific.VP9.height[i];
-          }
-        }
-        rtp->vp9().gof.CopyGofInfoVP9(info->codecSpecific.VP9.gof);
-      }
-
-      rtp->vp9().num_ref_pics = info->codecSpecific.VP9.num_ref_pics;
-      for (int i = 0; i < info->codecSpecific.VP9.num_ref_pics; ++i) {
-        rtp->vp9().pid_diff[i] = info->codecSpecific.VP9.p_diff[i];
-      }
-      rtp->vp9().end_of_picture = info->codecSpecific.VP9.end_of_picture;
-      return;
-    }
+    case kVideoCodecVP8:
+      return absl::optional<size_t>(info->codecSpecific.VP8.simulcastIdx);
     case kVideoCodecH264:
-      rtp->h264().packetization_mode =
-          info->codecSpecific.H264.packetization_mode;
-      rtp->simulcastIdx = info->codecSpecific.H264.simulcast_idx;
-      return;
+      return absl::optional<size_t>(info->codecSpecific.H264.simulcast_idx);
     case kVideoCodecMultiplex:
     case kVideoCodecGeneric:
-      rtp->codec = kVideoCodecGeneric;
-      rtp->simulcastIdx = info->codecSpecific.generic.simulcast_idx;
-      return;
+      return absl::optional<size_t>(info->codecSpecific.generic.simulcast_idx);
     default:
-      return;
-  }
-}
-
-void SetVideoTiming(VideoSendTiming* timing, const EncodedImage& image) {
-  if (image.timing_.flags == VideoSendTiming::TimingFrameFlags::kInvalid ||
-      image.timing_.flags == VideoSendTiming::TimingFrameFlags::kNotTriggered) {
-    timing->flags = VideoSendTiming::TimingFrameFlags::kInvalid;
-    return;
+      return absl::nullopt;
   }
-
-  timing->encode_start_delta_ms = VideoSendTiming::GetDeltaCappedMs(
-      image.capture_time_ms_, image.timing_.encode_start_ms);
-  timing->encode_finish_delta_ms = VideoSendTiming::GetDeltaCappedMs(
-      image.capture_time_ms_, image.timing_.encode_finish_ms);
-  timing->packetization_finish_delta_ms = 0;
-  timing->pacer_exit_delta_ms = 0;
-  timing->network_timestamp_delta_ms = 0;
-  timing->network2_timestamp_delta_ms = 0;
-  timing->flags = image.timing_.flags;
 }
-
 }  // namespace
 
-// State for setting picture id and tl0 pic idx, for VP8 and VP9
-// TODO(nisse): Make these properties not codec specific.
-class PayloadRouter::RtpPayloadParams final {
- public:
-  RtpPayloadParams(const uint32_t ssrc, const RtpPayloadState* state)
-      : ssrc_(ssrc) {
-    Random random(rtc::TimeMicros());
-    state_.picture_id =
-        state ? state->picture_id : (random.Rand<int16_t>() & 0x7FFF);
-    state_.tl0_pic_idx = state ? state->tl0_pic_idx : (random.Rand<uint8_t>());
-  }
-  ~RtpPayloadParams() {}
-
-  void Set(RTPVideoHeader* rtp_video_header, bool first_frame_in_picture) {
-    // Always set picture id. Set tl0_pic_idx iff temporal index is set.
-    if (first_frame_in_picture) {
-      state_.picture_id =
-          (static_cast<uint16_t>(state_.picture_id) + 1) & 0x7FFF;
-    }
-    if (rtp_video_header->codec == kVideoCodecVP8) {
-      rtp_video_header->vp8().pictureId = state_.picture_id;
-
-      if (rtp_video_header->vp8().temporalIdx != kNoTemporalIdx) {
-        if (rtp_video_header->vp8().temporalIdx == 0) {
-          ++state_.tl0_pic_idx;
-        }
-        rtp_video_header->vp8().tl0PicIdx = state_.tl0_pic_idx;
-      }
-    }
-    if (rtp_video_header->codec == kVideoCodecVP9) {
-      rtp_video_header->vp9().picture_id = state_.picture_id;
-
-      // Note that in the case that we have no temporal layers but we do have
-      // spatial layers, packets will carry layering info with a temporal_idx of
-      // zero, and we then have to set and increment tl0_pic_idx.
-      if (rtp_video_header->vp9().temporal_idx != kNoTemporalIdx ||
-          rtp_video_header->vp9().spatial_idx != kNoSpatialIdx) {
-        if (first_frame_in_picture &&
-            (rtp_video_header->vp9().temporal_idx == 0 ||
-             rtp_video_header->vp9().temporal_idx == kNoTemporalIdx)) {
-          ++state_.tl0_pic_idx;
-        }
-        rtp_video_header->vp9().tl0_pic_idx = state_.tl0_pic_idx;
-      }
-    }
-  }
-
-  uint32_t ssrc() const { return ssrc_; }
-
-  RtpPayloadState state() const { return state_; }
-
- private:
-  const uint32_t ssrc_;
-  RtpPayloadState state_;
-};
-
 PayloadRouter::PayloadRouter(const std::vector<RtpRtcp*>& rtp_modules,
                              const std::vector<uint32_t>& ssrcs,
                              int payload_type,
@@ -229,25 +101,10 @@ EncodedImageCallback::Result PayloadRouter::OnEncodedImage(
   if (!active_)
     return Result(Result::ERROR_SEND_FAILED);
 
-  RTPVideoHeader rtp_video_header;
-  if (codec_specific_info)
-    CopyCodecSpecific(codec_specific_info, &rtp_video_header);
-
-  rtp_video_header.rotation = encoded_image.rotation_;
-  rtp_video_header.content_type = encoded_image.content_type_;
-  rtp_video_header.playout_delay = encoded_image.playout_delay_;
-
-  SetVideoTiming(&rtp_video_header.video_timing, encoded_image);
-
-  int stream_index = rtp_video_header.simulcastIdx;
+  size_t stream_index = GetSimulcastIdx(codec_specific_info).value_or(0);
   RTC_DCHECK_LT(stream_index, rtp_modules_.size());
-
-  // Sets picture id and tl0 pic idx.
-  const bool first_frame_in_picture =
-      (codec_specific_info && codec_specific_info->codecType == kVideoCodecVP9)
-          ? codec_specific_info->codecSpecific.VP9.first_frame_in_picture
-          : true;
-  params_[stream_index].Set(&rtp_video_header, first_frame_in_picture);
+  RTPVideoHeader rtp_video_header = params_[stream_index].GetRtpVideoHeader(
+      encoded_image, codec_specific_info);
 
   uint32_t frame_id;
   if (!rtp_modules_[stream_index]->Sending()) {
@@ -272,22 +129,16 @@ void PayloadRouter::OnBitrateAllocationUpdated(
       // If spatial scalability is enabled, it is covered by a single stream.
       rtp_modules_[0]->SetVideoBitrateAllocation(bitrate);
     } else {
+      std::vector<absl::optional<VideoBitrateAllocation>> layer_bitrates =
+          bitrate.GetSimulcastAllocations();
       // Simulcast is in use, split the VideoBitrateAllocation into one struct
       // per rtp stream, moving over the temporal layer allocation.
-      for (size_t si = 0; si < rtp_modules_.size(); ++si) {
-        // Don't send empty TargetBitrate messages on streams not being relayed.
-        if (!bitrate.IsSpatialLayerUsed(si)) {
-          // The next spatial layer could be used if the current one is
-          // inactive.
-          continue;
-        }
-
-        VideoBitrateAllocation layer_bitrate;
-        for (int tl = 0; tl < kMaxTemporalStreams; ++tl) {
-          if (bitrate.HasBitrate(si, tl))
-            layer_bitrate.SetBitrate(0, tl, bitrate.GetBitrate(si, tl));
+      for (size_t i = 0; i < rtp_modules_.size(); ++i) {
+        // The next spatial layer could be used if the current one is
+        // inactive.
+        if (layer_bitrates[i]) {
+          rtp_modules_[i]->SetVideoBitrateAllocation(*layer_bitrates[i]);
         }
-        rtp_modules_[si]->SetVideoBitrateAllocation(layer_bitrate);
       }
     }
   }