VideoRtpReceiver: Enable encoded frame sink.

This change finally wires up VideoRtpReceiver::OnGenerateKeyFrame and
OnEncodedSinkEnabled into internal::VideoReceiveStream so that encoded
frames can flow to sinks installed in VideoTrackSourceInterface.

Bug: chromium:1013590
Change-Id: I76f8226752294aee8fe137d1a78ee66548900cc2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/161095
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30003}

api/video/test/BUILD.gn

@@ -22,3 +22,16 @@ rtc_library("rtc_api_video_unittests") {
     "//third_party/abseil-cpp/absl/types:optional",
   ]
 }
+
+rtc_source_set("mock_recordable_encoded_frame") {
+  testonly = true
+  visibility = [ "*" ]
+  sources = [
+    "mock_recordable_encoded_frame.h",
+  ]
+
+  deps = [
+    "..:recordable_encoded_frame",
+    "../../../test:test_support",
+  ]
+}

api/video/test/mock_recordable_encoded_frame.h

@@ -0,0 +1,29 @@
+/*
+ *  Copyright (c) 2019 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.
+ */
+
+#ifndef API_VIDEO_TEST_MOCK_RECORDABLE_ENCODED_FRAME_H_
+#define API_VIDEO_TEST_MOCK_RECORDABLE_ENCODED_FRAME_H_
+
+#include "api/video/recordable_encoded_frame.h"
+#include "test/gmock.h"
+
+namespace webrtc {
+class MockRecordableEncodedFrame : public RecordableEncodedFrame {
+ public:
+  MOCK_CONST_METHOD0(encoded_buffer,
+                     rtc::scoped_refptr<const EncodedImageBufferInterface>());
+  MOCK_CONST_METHOD0(color_space, absl::optional<webrtc::ColorSpace>());
+  MOCK_CONST_METHOD0(codec, VideoCodecType());
+  MOCK_CONST_METHOD0(is_key_frame, bool());
+  MOCK_CONST_METHOD0(resolution, EncodedResolution());
+  MOCK_CONST_METHOD0(render_time, Timestamp());
+};
+}  // namespace webrtc
+#endif  // API_VIDEO_TEST_MOCK_RECORDABLE_ENCODED_FRAME_H_

pc/BUILD.gn

@@ -314,6 +314,7 @@ if (rtc_include_tests) {
       "test/rtp_transport_test_util.h",
       "test/srtp_test_util.h",
       "used_ids_unittest.cc",
+      "video_rtp_receiver_unittest.cc",
     ]
 
     include_dirs = [ "//third_party/libsrtp/srtp" ]
@@ -325,6 +326,7 @@ if (rtc_include_tests) {
     deps = [
       ":libjingle_peerconnection",
       ":pc_test_utils",
+      ":peerconnection",
       ":rtc_pc",
       ":rtc_pc_base",
       "../api:array_view",
@@ -338,6 +340,7 @@ if (rtc_include_tests) {
       "../api:rtp_parameters",
       "../api/transport/media:media_transport_interface",
       "../api/video:builtin_video_bitrate_allocator_factory",
+      "../api/video/test:mock_recordable_encoded_frame",
       "../call:rtp_interfaces",
       "../call:rtp_receiver",
       "../media:rtc_data",

pc/video_rtp_receiver.cc

@@ -78,15 +78,6 @@ std::vector<std::string> VideoRtpReceiver::stream_ids() const {
   return stream_ids;
 }
 
-bool VideoRtpReceiver::SetSink(rtc::VideoSinkInterface<VideoFrame>* sink) {
-  RTC_DCHECK(media_channel_);
-  RTC_DCHECK(!stopped_);
-  return worker_thread_->Invoke<bool>(RTC_FROM_HERE, [&] {
-    // TODO(bugs.webrtc.org/8694): Stop using 0 to mean unsignalled SSRC
-    return media_channel_->SetSink(ssrc_.value_or(0), sink);
-  });
-}
-
 RtpParameters VideoRtpReceiver::GetParameters() const {
   if (!media_channel_ || stopped_) {
     return RtpParameters();
@@ -122,9 +113,12 @@ void VideoRtpReceiver::Stop() {
   if (!media_channel_) {
     RTC_LOG(LS_WARNING) << "VideoRtpReceiver::Stop: No video channel exists.";
   } else {
-    // Allow that SetSink fail. This is the normal case when the underlying
+    // Allow that SetSink fails. This is the normal case when the underlying
     // media channel has already been deleted.
-    SetSink(nullptr);
+    worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
+      RTC_DCHECK_RUN_ON(worker_thread_);
+      SetSink(nullptr);
+    });
   }
   delay_->OnStop();
   stopped_ = true;
@@ -135,12 +129,22 @@ void VideoRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
   if (!stopped_ && ssrc_ == ssrc) {
     return;
   }
-  if (!stopped_) {
-    SetSink(nullptr);
-  }
-  stopped_ = false;
-  ssrc_ = ssrc;
-  SetSink(source_->sink());
+  worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
+    RTC_DCHECK_RUN_ON(worker_thread_);
+    if (!stopped_) {
+      SetSink(nullptr);
+    }
+    bool encoded_sink_enabled = saved_encoded_sink_enabled_;
+    SetEncodedSinkEnabled(false);
+    stopped_ = false;
+
+    ssrc_ = ssrc;
+
+    SetSink(source_->sink());
+    if (encoded_sink_enabled) {
+      SetEncodedSinkEnabled(true);
+    }
+  });
 
   // Attach any existing frame decryptor to the media channel.
   MaybeAttachFrameDecryptorToMediaChannel(
@@ -150,6 +154,11 @@ void VideoRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
   delay_->OnStart(media_channel_, ssrc.value_or(0));
 }
 
+void VideoRtpReceiver::SetSink(rtc::VideoSinkInterface<VideoFrame>* sink) {
+  // TODO(bugs.webrtc.org/8694): Stop using 0 to mean unsignalled SSRC
+  media_channel_->SetSink(ssrc_.value_or(0), sink);
+}
+
 void VideoRtpReceiver::SetupMediaChannel(uint32_t ssrc) {
   if (!media_channel_) {
     RTC_LOG(LS_ERROR)
@@ -219,7 +228,27 @@ void VideoRtpReceiver::SetJitterBufferMinimumDelay(
 void VideoRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) {
   RTC_DCHECK(media_channel == nullptr ||
              media_channel->media_type() == media_type());
-  media_channel_ = static_cast<cricket::VideoMediaChannel*>(media_channel);
+  worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
+    RTC_DCHECK_RUN_ON(worker_thread_);
+    bool encoded_sink_enabled = saved_encoded_sink_enabled_;
+    if (encoded_sink_enabled && media_channel_) {
+      // Turn off the old sink, if any.
+      SetEncodedSinkEnabled(false);
+    }
+
+    media_channel_ = static_cast<cricket::VideoMediaChannel*>(media_channel);
+
+    if (media_channel_) {
+      if (saved_generate_keyframe_) {
+        // TODO(bugs.webrtc.org/8694): Stop using 0 to mean unsignalled SSRC
+        media_channel_->GenerateKeyFrame(ssrc_.value_or(0));
+        saved_generate_keyframe_ = false;
+      }
+      if (encoded_sink_enabled) {
+        SetEncodedSinkEnabled(true);
+      }
+    }
+  });
 }
 
 void VideoRtpReceiver::NotifyFirstPacketReceived() {
@@ -239,10 +268,37 @@ std::vector<RtpSource> VideoRtpReceiver::GetSources() const {
 
 void VideoRtpReceiver::OnGenerateKeyFrame() {
   RTC_DCHECK_RUN_ON(worker_thread_);
+  // TODO(bugs.webrtc.org/8694): Stop using 0 to mean unsignalled SSRC
+  media_channel_->GenerateKeyFrame(ssrc_.value_or(0));
+  // We need to remember to request generation of a new key frame if the media
+  // channel changes, because there's no feedback whether the keyframe
+  // generation has completed on the channel.
+  saved_generate_keyframe_ = true;
 }
 
 void VideoRtpReceiver::OnEncodedSinkEnabled(bool enable) {
   RTC_DCHECK_RUN_ON(worker_thread_);
+  SetEncodedSinkEnabled(enable);
+  // Always save the latest state of the callback in case the media_channel_
+  // changes.
+  saved_encoded_sink_enabled_ = enable;
+}
+
+void VideoRtpReceiver::SetEncodedSinkEnabled(bool enable) {
+  if (media_channel_) {
+    if (enable) {
+      // TODO(bugs.webrtc.org/8694): Stop using 0 to mean unsignalled SSRC
+      auto source = source_;
+      media_channel_->SetRecordableEncodedFrameCallback(
+          ssrc_.value_or(0),
+          [source = std::move(source)](const RecordableEncodedFrame& frame) {
+            source->BroadcastRecordableEncodedFrame(frame);
+          });
+    } else {
+      // TODO(bugs.webrtc.org/8694): Stop using 0 to mean unsignalled SSRC
+      media_channel_->ClearRecordableEncodedFrameCallback(ssrc_.value_or(0));
+    }
+  }
 }
 
 }  // namespace webrtc

pc/video_rtp_receiver.h

@@ -110,11 +110,13 @@ class VideoRtpReceiver : public rtc::RefCountedObject<RtpReceiverInternal>,
 
  private:
   void RestartMediaChannel(absl::optional<uint32_t> ssrc);
-  bool SetSink(rtc::VideoSinkInterface<VideoFrame>* sink);
+  void SetSink(rtc::VideoSinkInterface<VideoFrame>* sink)
+      RTC_RUN_ON(worker_thread_);
 
   // VideoRtpTrackSource::Callback
   void OnGenerateKeyFrame() override;
   void OnEncodedSinkEnabled(bool enable) override;
+  void SetEncodedSinkEnabled(bool enable) RTC_RUN_ON(worker_thread_);
 
   rtc::Thread* const worker_thread_;
 
@@ -135,6 +137,10 @@ class VideoRtpReceiver : public rtc::RefCountedObject<RtpReceiverInternal>,
   // Allows to thread safely change jitter buffer delay. Handles caching cases
   // if |SetJitterBufferMinimumDelay| is called before start.
   rtc::scoped_refptr<JitterBufferDelayInterface> delay_;
+  // Records if we should generate a keyframe when |media_channel_| gets set up
+  // or switched.
+  bool saved_generate_keyframe_ RTC_GUARDED_BY(worker_thread_) = false;
+  bool saved_encoded_sink_enabled_ RTC_GUARDED_BY(worker_thread_) = false;
 };
 
 }  // namespace webrtc

pc/video_rtp_receiver_unittest.cc

@@ -0,0 +1,160 @@
+/*
+ *  Copyright 2019 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 "pc/video_rtp_receiver.h"
+
+#include <memory>
+
+#include "api/video/test/mock_recordable_encoded_frame.h"
+#include "media/base/fake_media_engine.h"
+#include "test/gmock.h"
+
+using ::testing::_;
+using ::testing::InSequence;
+using ::testing::Mock;
+using ::testing::SaveArg;
+using ::testing::StrictMock;
+
+namespace webrtc {
+namespace {
+
+class VideoRtpReceiverTest : public testing::Test {
+ protected:
+  class MockVideoMediaChannel : public cricket::FakeVideoMediaChannel {
+   public:
+    MockVideoMediaChannel(cricket::FakeVideoEngine* engine,
+                          const cricket::VideoOptions& options)
+        : FakeVideoMediaChannel(engine, options) {}
+    MOCK_METHOD2(SetRecordableEncodedFrameCallback,
+                 void(uint32_t,
+                      std::function<void(const RecordableEncodedFrame&)>));
+    MOCK_METHOD1(ClearRecordableEncodedFrameCallback, void(uint32_t));
+    MOCK_METHOD1(GenerateKeyFrame, void(uint32_t));
+  };
+
+  class MockVideoSink : public rtc::VideoSinkInterface<RecordableEncodedFrame> {
+   public:
+    MOCK_METHOD1(OnFrame, void(const RecordableEncodedFrame&));
+  };
+
+  VideoRtpReceiverTest()
+      : worker_thread_(rtc::Thread::Create()),
+        channel_(nullptr, cricket::VideoOptions()),
+        receiver_(new VideoRtpReceiver(worker_thread_.get(),
+                                       "receiver",
+                                       {"stream"})) {
+    worker_thread_->Start();
+    receiver_->SetMediaChannel(&channel_);
+  }
+
+  webrtc::VideoTrackSourceInterface* Source() {
+    return receiver_->streams()[0]->FindVideoTrack("receiver")->GetSource();
+  }
+
+  std::unique_ptr<rtc::Thread> worker_thread_;
+  MockVideoMediaChannel channel_;
+  rtc::scoped_refptr<VideoRtpReceiver> receiver_;
+};
+
+TEST_F(VideoRtpReceiverTest, SupportsEncodedOutput) {
+  EXPECT_TRUE(Source()->SupportsEncodedOutput());
+}
+
+TEST_F(VideoRtpReceiverTest, GeneratesKeyFrame) {
+  EXPECT_CALL(channel_, GenerateKeyFrame(0));
+  Source()->GenerateKeyFrame();
+}
+
+TEST_F(VideoRtpReceiverTest,
+       GenerateKeyFrameOnChannelSwitchUnlessGenerateKeyframeCalled) {
+  // A channel switch without previous call to GenerateKeyFrame shouldn't
+  // cause a call to happen on the new channel.
+  MockVideoMediaChannel channel2(nullptr, cricket::VideoOptions());
+  EXPECT_CALL(channel_, GenerateKeyFrame).Times(0);
+  EXPECT_CALL(channel2, GenerateKeyFrame).Times(0);
+  receiver_->SetMediaChannel(&channel2);
+  Mock::VerifyAndClearExpectations(&channel2);
+
+  // Generate a key frame. When we switch channel next time, we will have to
+  // re-generate it as we don't know if it was eventually received
+  Source()->GenerateKeyFrame();
+  MockVideoMediaChannel channel3(nullptr, cricket::VideoOptions());
+  EXPECT_CALL(channel3, GenerateKeyFrame);
+  receiver_->SetMediaChannel(&channel3);
+
+  // Switching to a new channel should now not cause calls to GenerateKeyFrame.
+  StrictMock<MockVideoMediaChannel> channel4(nullptr, cricket::VideoOptions());
+  receiver_->SetMediaChannel(&channel4);
+}
+
+TEST_F(VideoRtpReceiverTest, EnablesEncodedOutput) {
+  EXPECT_CALL(channel_, SetRecordableEncodedFrameCallback(/*ssrc=*/0, _));
+  EXPECT_CALL(channel_, ClearRecordableEncodedFrameCallback).Times(0);
+  MockVideoSink sink;
+  Source()->AddEncodedSink(&sink);
+}
+
+TEST_F(VideoRtpReceiverTest, DisablesEncodedOutput) {
+  EXPECT_CALL(channel_, ClearRecordableEncodedFrameCallback(/*ssrc=*/0));
+  MockVideoSink sink;
+  Source()->AddEncodedSink(&sink);
+  Source()->RemoveEncodedSink(&sink);
+}
+
+TEST_F(VideoRtpReceiverTest, DisablesEnablesEncodedOutputOnChannelSwitch) {
+  InSequence s;
+  EXPECT_CALL(channel_, SetRecordableEncodedFrameCallback);
+  EXPECT_CALL(channel_, ClearRecordableEncodedFrameCallback);
+  MockVideoSink sink;
+  Source()->AddEncodedSink(&sink);
+  MockVideoMediaChannel channel2(nullptr, cricket::VideoOptions());
+  EXPECT_CALL(channel2, SetRecordableEncodedFrameCallback);
+  receiver_->SetMediaChannel(&channel2);
+  Mock::VerifyAndClearExpectations(&channel2);
+
+  // When clearing encoded frame buffer function, we need channel switches
+  // to NOT set the callback again.
+  EXPECT_CALL(channel2, ClearRecordableEncodedFrameCallback);
+  Source()->RemoveEncodedSink(&sink);
+  StrictMock<MockVideoMediaChannel> channel3(nullptr, cricket::VideoOptions());
+  receiver_->SetMediaChannel(&channel3);
+}
+
+TEST_F(VideoRtpReceiverTest, BroadcastsEncodedFramesWhenEnabled) {
+  std::function<void(const RecordableEncodedFrame&)> broadcast;
+  EXPECT_CALL(channel_, SetRecordableEncodedFrameCallback(_, _))
+      .WillRepeatedly(SaveArg<1>(&broadcast));
+  MockVideoSink sink;
+  Source()->AddEncodedSink(&sink);
+
+  // Make sure SetEncodedFrameBufferFunction completes.
+  Mock::VerifyAndClearExpectations(&channel_);
+
+  // Pass two frames on different contexts.
+  EXPECT_CALL(sink, OnFrame).Times(2);
+  MockRecordableEncodedFrame frame;
+  broadcast(frame);
+  worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] { broadcast(frame); });
+}
+
+TEST_F(VideoRtpReceiverTest, EnablesEncodedOutputOnChannelRestart) {
+  InSequence s;
+  EXPECT_CALL(channel_, ClearRecordableEncodedFrameCallback(0));
+  MockVideoSink sink;
+  Source()->AddEncodedSink(&sink);
+  EXPECT_CALL(channel_, SetRecordableEncodedFrameCallback(4711, _));
+  receiver_->SetupMediaChannel(4711);
+  EXPECT_CALL(channel_, ClearRecordableEncodedFrameCallback(4711));
+  EXPECT_CALL(channel_, SetRecordableEncodedFrameCallback(0, _));
+  receiver_->SetupUnsignaledMediaChannel();
+}
+
+}  // namespace
+}  // namespace webrtc