Add callback for providing feedback for video capture in Render process

This CL makes all the relevant tracks/capturers to return a feedback
callback upon connection to the source or starting the capture.
The call chain results in VideoCaptureImpl method being plumbed to the
client as a callback.

This is the first step for removing VideoFrameFeedback from the
media::VideoFrame.

Follow-up CLs will introduce the callbacks to mirroring service (which is
a second user of feedback mechanism) and would remove VideoFrameFeedback
member from media::VideoFrame.

Bug: chromium:1134073
Change-Id: I61dfc8449e69ad53f3fc6610d8cfbf4cdb6e4441
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489644
Commit-Queue: Ilya Nikolaevskiy <ilnik@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Auto-Submit: Ilya Nikolaevskiy <ilnik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825316}

media/base/video_frame_feedback.h

@@ -7,10 +7,16 @@
 
 #include <limits>
 
+#include "base/callback.h"
 #include "media/base/media_export.h"
 
 namespace media {
 
+struct VideoFrameFeedback;
+
+using VideoCaptureFeedbackCB =
+    base::RepeatingCallback<void(const VideoFrameFeedback&)>;
+
 // Feedback from the frames consumer.
 // This class is passed from the frames sink to the capturer to limit
 // incoming video feed frame-rate and/or resolution.

media/capture/video_capturer_source.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "media/capture/video_capturer_source.h"
+#include "base/bind_helpers.h"
 
 namespace media {
 
@@ -13,4 +14,8 @@ namespace media {
 // to generate symbols across linking units.
 VideoCapturerSource::~VideoCapturerSource() = default;
 
+media::VideoCaptureFeedbackCB VideoCapturerSource::GetFeedbackCallback() const {
+  return base::DoNothing();
+}
+
 }  // namespace media

media/capture/video_capturer_source.h

@@ -11,6 +11,7 @@
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/time/time.h"
+#include "media/base/video_frame_feedback.h"
 #include "media/capture/capture_export.h"
 #include "media/capture/video_capture_types.h"
 
@@ -69,6 +70,10 @@ class CAPTURE_EXPORT VideoCapturerSource {
       const VideoCaptureDeliverFrameCB& new_frame_callback,
       const RunningCallback& running_callback) = 0;
 
+  // Returns a callback for providing the feedback from the consumer.
+  // The callback can be called on any thread.
+  virtual media::VideoCaptureFeedbackCB GetFeedbackCallback() const;
+
   // Asks source to send a refresh frame. In cases where source does not provide
   // a continuous rate of new frames (e.g. canvas capture, screen capture where
   // the screen's content has not changed in a while), consumers may request a

third_party/blink/public/common/media/video_capture.h

@@ -8,6 +8,7 @@
 #define THIRD_PARTY_BLINK_PUBLIC_COMMON_MEDIA_VIDEO_CAPTURE_H_
 
 #include "base/callback.h"
+#include "media/base/video_frame_feedback.h"
 #include "media/capture/video_capture_types.h"
 #include "media/capture/video_capturer_source.h"
 
@@ -19,6 +20,8 @@ using VideoCaptureDeviceFormatsCB =
 using VideoCaptureDeliverFrameCB =
     media::VideoCapturerSource::VideoCaptureDeliverFrameCB;
 
+using VideoCaptureFeedbackCB = media::VideoCaptureFeedbackCB;
+
 // Current status of the video capture device. It's used by multiple classes in
 // browser process and renderer process. Browser process sends information about
 // the current capture state and error to the renderer process using this type.

third_party/blink/public/platform/modules/video_capture/web_video_capture_impl_manager.h

@@ -107,10 +107,22 @@ class BLINK_PLATFORM_EXPORT WebVideoCaptureImplManager {
   virtual std::unique_ptr<VideoCaptureImpl> CreateVideoCaptureImplForTesting(
       const media::VideoCaptureSessionId& session_id) const;
 
+  // Get the feedback callback for the corresponding capture session.
+  // Consumers may call the returned callback in any thread to provide
+  // the capturer with feedback information.
+  VideoCaptureFeedbackCB GetFeedbackCallback(
+      const media::VideoCaptureSessionId& id) const;
+
  private:
   // Holds bookkeeping info for each VideoCaptureImpl shared by clients.
   struct DeviceEntry;
 
+  static void ProcessFeedback(VideoCaptureFeedbackCB callback_to_io_thread,
+                              const media::VideoFrameFeedback& feedback);
+
+  void ProcessFeedbackInternal(const media::VideoCaptureSessionId& id,
+                               const media::VideoFrameFeedback& feedback);
+
   void StopCapture(int client_id, const media::VideoCaptureSessionId& id);
   void UnrefDevice(const media::VideoCaptureSessionId& id);
 

third_party/blink/public/web/modules/mediastream/media_stream_video_source.h

@@ -172,6 +172,11 @@ class BLINK_MODULES_EXPORT MediaStreamVideoSource
 
   bool IsStoppedForRestart() const { return state_ == STOPPED_FOR_RESTART; }
 
+  // Provides a callback for consumers to trigger when they have some
+  // feedback to report.
+  // The returned callback can be called on any thread.
+  virtual VideoCaptureFeedbackCB GetFeedbackCallback() const;
+
   size_t NumTracks() const {
     DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
     return tracks_.size();

third_party/blink/renderer/modules/mediastream/media_stream_video_capturer_source.cc

@@ -97,6 +97,11 @@ void MediaStreamVideoCapturerSource::StartSourceImpl(
                          WTF::Unretained(this), capture_params_));
 }
 
+media::VideoCaptureFeedbackCB
+MediaStreamVideoCapturerSource::GetFeedbackCallback() const {
+  return source_->GetFeedbackCallback();
+}
+
 void MediaStreamVideoCapturerSource::StopSourceImpl() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   source_->StopCapture();

third_party/blink/renderer/modules/mediastream/media_stream_video_capturer_source.h

@@ -73,6 +73,7 @@ class MODULES_EXPORT MediaStreamVideoCapturerSource
   void OnCapturingLinkSecured(bool is_secure) override;
   void StartSourceImpl(VideoCaptureDeliverFrameCB frame_callback,
                        EncodedVideoFrameCB encoded_frame_callback) override;
+  media::VideoCaptureFeedbackCB GetFeedbackCallback() const override;
   void StopSourceImpl() override;
   void StopSourceForRestartImpl() override;
   void RestartSourceImpl(const media::VideoCaptureFormat& new_format) override;

third_party/blink/renderer/modules/mediastream/media_stream_video_source.cc

@@ -508,6 +508,11 @@ bool MediaStreamVideoSource::SupportsEncodedOutput() const {
   return false;
 }
 
+VideoCaptureFeedbackCB MediaStreamVideoSource::GetFeedbackCallback() const {
+  // Each source implementation has to implement its own feedback callbacks.
+  return base::DoNothing();
+}
+
 MediaStreamVideoSource::PendingTrackInfo::PendingTrackInfo(
     MediaStreamVideoTrack* track,
     const VideoCaptureDeliverFrameCB& frame_callback,

third_party/blink/renderer/modules/peerconnection/media_stream_video_webrtc_sink.cc

@@ -172,8 +172,9 @@ MediaStreamVideoWebRtcSink::MediaStreamVideoWebRtcSink(
   // TODO(pbos): Consolidate WebRtcVideoCapturerAdapter into WebRtcVideoSource
   // by removing the need for and dependency on a cricket::VideoCapturer.
   video_source_ = scoped_refptr<WebRtcVideoTrackSource>(
-      new rtc::RefCountedObject<WebRtcVideoTrackSource>(is_screencast,
-                                                        needs_denoising));
+      new rtc::RefCountedObject<WebRtcVideoTrackSource>(
+          is_screencast, needs_denoising,
+          video_track->source()->GetFeedbackCallback()));
 
   // TODO(pbos): Consolidate the local video track with the source proxy and
   // move into PeerConnectionDependencyFactory. This now separately holds on a

third_party/blink/renderer/platform/exported/video_capture/web_video_capture_impl_manager.cc

@@ -30,6 +30,7 @@
 #include "base/bind_helpers.h"
 #include "base/location.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "media/base/bind_to_current_loop.h"
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/public/platform/web_string.h"
 #include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h"
@@ -315,12 +316,47 @@ void WebVideoCaptureImplManager::OnLog(const media::VideoCaptureSessionId& id,
       devices_.begin(), devices_.end(),
       [id](const DeviceEntry& entry) { return entry.session_id == id; });
   DCHECK(it != devices_.end());
-  // Use of base::Unretained() is safe because |devices_| is released on the
-  // |io_task_runner()| as well.
+  // Use of base::CrossthreadUnretained() is safe because |devices_| is released
+  // on the |io_task_runner()| as well.
   PostCrossThreadTask(*Platform::Current()->GetIOTaskRunner().get(), FROM_HERE,
                       CrossThreadBindOnce(&VideoCaptureImpl::OnLog,
                                           CrossThreadUnretained(it->impl.get()),
                                           String(message)));
 }
 
+VideoCaptureFeedbackCB WebVideoCaptureImplManager::GetFeedbackCallback(
+    const media::VideoCaptureSessionId& id) const {
+  DCHECK(render_main_task_runner_->BelongsToCurrentThread());
+  return base::BindRepeating(
+      &WebVideoCaptureImplManager::ProcessFeedback,
+      media::BindToCurrentLoop(base::BindRepeating(
+          &WebVideoCaptureImplManager::ProcessFeedbackInternal,
+          weak_factory_.GetWeakPtr(), id)));
+}
+
+// static
+void WebVideoCaptureImplManager::ProcessFeedback(
+    VideoCaptureFeedbackCB callback_to_io_thread,
+    const media::VideoFrameFeedback& feedback) {
+  // process feedback can be called on any thread by the client.
+  callback_to_io_thread.Run(feedback);
+}
+
+void WebVideoCaptureImplManager::ProcessFeedbackInternal(
+    const media::VideoCaptureSessionId& id,
+    const media::VideoFrameFeedback& feedback) {
+  DCHECK(render_main_task_runner_->BelongsToCurrentThread());
+  const auto it = std::find_if(
+      devices_.begin(), devices_.end(),
+      [id](const DeviceEntry& entry) { return entry.session_id == id; });
+  if (it != devices_.end()) {
+    // Use of base::CrossthreadUnretained() is safe because |devices_| is
+    // released on the |io_task_runner()| as well.
+    PostCrossThreadTask(
+        *Platform::Current()->GetIOTaskRunner().get(), FROM_HERE,
+        CrossThreadBindOnce(&VideoCaptureImpl::ProcessFeedback,
+                            CrossThreadUnretained(it->impl.get()), feedback));
+  }
+}
+
 }  // namespace blink

third_party/blink/renderer/platform/peerconnection/webrtc_video_track_source.cc

@@ -87,11 +87,13 @@ namespace blink {
 
 WebRtcVideoTrackSource::WebRtcVideoTrackSource(
     bool is_screencast,
-    absl::optional<bool> needs_denoising)
+    absl::optional<bool> needs_denoising,
+    media::VideoCaptureFeedbackCB callback)
     : AdaptedVideoTrackSource(/*required_alignment=*/1),
       scaled_frame_pool_(new WebRtcVideoFrameAdapter::BufferPoolOwner()),
       is_screencast_(is_screencast),
-      needs_denoising_(needs_denoising) {
+      needs_denoising_(needs_denoising),
+      callback_(callback) {
   DETACH_FROM_THREAD(thread_checker_);
 }
 
@@ -124,11 +126,15 @@ absl::optional<bool> WebRtcVideoTrackSource::needs_denoising() const {
   return needs_denoising_;
 }
 
-void WebRtcVideoTrackSource::SetFrameFeedback(
-    scoped_refptr<media::VideoFrame> frame) {
-  media::VideoFrameFeedback* feedback = frame->feedback();
-  feedback->max_pixels = video_adapter()->GetTargetPixels();
-  feedback->max_framerate_fps = video_adapter()->GetMaxFramerate();
+void WebRtcVideoTrackSource::SendFeedback() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  if (callback_.is_null()) {
+    return;
+  }
+  media::VideoFrameFeedback feedback;
+  feedback.max_pixels = video_adapter()->GetTargetPixels();
+  feedback.max_framerate_fps = video_adapter()->GetMaxFramerate();
+  callback_.Run(feedback);
 }
 
 void WebRtcVideoTrackSource::OnFrameCaptured(
@@ -149,7 +155,7 @@ void WebRtcVideoTrackSource::OnFrameCaptured(
     return;
   }
 
-  SetFrameFeedback(frame);
+  SendFeedback();
 
   // Compute what rectangular region has changed since the last frame
   // that we successfully delivered to the base class method

third_party/blink/renderer/platform/peerconnection/webrtc_video_track_source.h

@@ -8,6 +8,7 @@
 #include "base/feature_list.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/threading/thread_checker.h"
+#include "media/base/video_frame_feedback.h"
 #include "media/base/video_frame_pool.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
 #include "third_party/blink/renderer/platform/webrtc/webrtc_video_frame_adapter.h"
@@ -34,7 +35,8 @@ class PLATFORM_EXPORT WebRtcVideoTrackSource
   };
 
   WebRtcVideoTrackSource(bool is_screencast,
-                         absl::optional<bool> needs_denoising);
+                         absl::optional<bool> needs_denoising,
+                         media::VideoCaptureFeedbackCB callback);
   ~WebRtcVideoTrackSource() override;
 
   void SetCustomFrameAdaptationParamsForTesting(
@@ -53,7 +55,7 @@ class PLATFORM_EXPORT WebRtcVideoTrackSource
   using webrtc::VideoTrackSourceInterface::RemoveSink;
 
  private:
-  void SetFrameFeedback(scoped_refptr<media::VideoFrame> frame);
+  void SendFeedback();
 
   FrameAdaptationParams ComputeAdaptationParams(int width,
                                                 int height,
@@ -86,6 +88,8 @@ class PLATFORM_EXPORT WebRtcVideoTrackSource
   absl::optional<FrameAdaptationParams>
       custom_frame_adaptation_params_for_testing_;
 
+  const media::VideoCaptureFeedbackCB callback_;
+
   DISALLOW_COPY_AND_ASSIGN(WebRtcVideoTrackSource);
 };
 

third_party/blink/renderer/platform/peerconnection/webrtc_video_track_source_test.cc

@@ -40,9 +40,16 @@ class WebRtcVideoTrackSourceTest
   WebRtcVideoTrackSourceTest()
       : track_source_(new rtc::RefCountedObject<WebRtcVideoTrackSource>(
             /*is_screencast=*/false,
-            /*needs_denoising=*/absl::nullopt)) {
+            /*needs_denoising=*/absl::nullopt,
+            base::BindRepeating(&WebRtcVideoTrackSourceTest::ProcessFeedback,
+                                base::Unretained(this)))) {
     track_source_->AddOrUpdateSink(&mock_sink_, rtc::VideoSinkWants());
   }
+
+  void ProcessFeedback(const media::VideoFrameFeedback& feedback) {
+    feedback_ = feedback;
+  }
+
   ~WebRtcVideoTrackSourceTest() override {
     track_source_->RemoveSink(&mock_sink_);
   }
@@ -66,8 +73,8 @@ class WebRtcVideoTrackSourceTest
     scoped_refptr<media::VideoFrame> frame =
         CreateTestFrame(coded_size, visible_rect, natural_size, storage_type);
     track_source_->OnFrameCaptured(frame);
-    EXPECT_EQ(frame->feedback()->max_pixels, max_pixels);
-    EXPECT_EQ(frame->feedback()->max_framerate_fps, max_framerate);
+    EXPECT_EQ(feedback_.max_pixels, max_pixels);
+    EXPECT_EQ(feedback_.max_framerate_fps, max_framerate);
   }
 
   void SendTestFrameWithUpdateRect(
@@ -126,6 +133,7 @@ class WebRtcVideoTrackSourceTest
  protected:
   MockVideoSink mock_sink_;
   scoped_refptr<WebRtcVideoTrackSource> track_source_;
+  media::VideoFrameFeedback feedback_;
 };
 
 TEST_P(WebRtcVideoTrackSourceTest, CropFrameTo640360) {

third_party/blink/renderer/platform/video_capture/local_video_capturer_source.cc

@@ -49,6 +49,11 @@ void LocalVideoCapturerSource::StartCapture(
       new_frame_callback);
 }
 
+media::VideoCaptureFeedbackCB LocalVideoCapturerSource::GetFeedbackCallback()
+    const {
+  return manager_->GetFeedbackCallback(session_id_);
+}
+
 void LocalVideoCapturerSource::RequestRefreshFrame() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   if (!stop_capture_cb_)

third_party/blink/renderer/platform/video_capture/local_video_capturer_source.h

@@ -54,6 +54,7 @@ class PLATFORM_EXPORT LocalVideoCapturerSource
   void StopCapture() override;
   void OnFrameDropped(media::VideoCaptureFrameDropReason reason) override;
   void OnLog(const std::string& message) override;
+  media::VideoCaptureFeedbackCB GetFeedbackCallback() const override;
 
  private:
   void OnStateUpdate(blink::VideoCaptureState state);

third_party/blink/renderer/platform/video_capture/video_capture_impl.cc

@@ -735,7 +735,7 @@ void VideoCaptureImpl::OnVideoFrameReady(
   }
 
   frame->AddDestructionObserver(base::BindOnce(
-      &VideoCaptureImpl::DidFinishConsumingFrame, frame->feedback(),
+      &VideoCaptureImpl::DidFinishConsumingFrame,
       media::BindToCurrentLoop(base::BindOnce(
           &VideoCaptureImpl::OnAllClientsFinishedConsumingFrame,
           weak_factory_.GetWeakPtr(), buffer_id, std::move(buffer_context)))));
@@ -767,8 +767,7 @@ constexpr base::TimeDelta VideoCaptureImpl::kCaptureStartTimeout;
 
 void VideoCaptureImpl::OnAllClientsFinishedConsumingFrame(
     int buffer_id,
-    scoped_refptr<BufferContext> buffer_context,
-    const media::VideoFrameFeedback feedback) {
+    scoped_refptr<BufferContext> buffer_context) {
   DCHECK_CALLED_ON_VALID_THREAD(io_thread_checker_);
 
 // Subtle race note: It's important that the |buffer_context| argument be
@@ -796,7 +795,8 @@ void VideoCaptureImpl::OnAllClientsFinishedConsumingFrame(
   buffer_context = nullptr;
 #endif
 
-  GetVideoCaptureHost()->ReleaseBuffer(device_id_, buffer_id, feedback);
+  GetVideoCaptureHost()->ReleaseBuffer(device_id_, buffer_id, feedback_);
+  feedback_ = media::VideoFrameFeedback();
 }
 
 void VideoCaptureImpl::StopDevice() {
@@ -889,11 +889,16 @@ media::mojom::blink::VideoCaptureHost* VideoCaptureImpl::GetVideoCaptureHost() {
 
 // static
 void VideoCaptureImpl::DidFinishConsumingFrame(
-    const media::VideoFrameFeedback* feedback,
     BufferFinishedCallback callback_to_io_thread) {
   // Note: This function may be called on any thread by the VideoFrame
   // destructor.  |metadata| is still valid for read-access at this point.
-  std::move(callback_to_io_thread).Run(*feedback);
+  std::move(callback_to_io_thread).Run();
+}
+
+void VideoCaptureImpl::ProcessFeedback(
+    const media::VideoFrameFeedback& feedback) {
+  DCHECK_CALLED_ON_VALID_THREAD(io_thread_checker_);
+  feedback_ = feedback;
 }
 
 }  // namespace blink

third_party/blink/renderer/platform/video_capture/video_capture_impl.h

@@ -100,6 +100,8 @@ class PLATFORM_EXPORT VideoCaptureImpl
                      media::mojom::blink::VideoFrameInfoPtr info) override;
   void OnBufferDestroyed(int32_t buffer_id) override;
 
+  void ProcessFeedback(const media::VideoFrameFeedback& feedback);
+
   static constexpr base::TimeDelta kCaptureStartTimeout =
       base::TimeDelta::FromSeconds(10);
 
@@ -114,8 +116,7 @@ class PLATFORM_EXPORT VideoCaptureImpl
   struct ClientInfo;
   using ClientInfoMap = std::map<int, ClientInfo>;
 
-  using BufferFinishedCallback =
-      base::OnceCallback<void(media::VideoFrameFeedback feedback)>;
+  using BufferFinishedCallback = base::OnceClosure;
 
   void OnVideoFrameReady(int32_t buffer_id,
                          base::TimeTicks reference_time,
@@ -125,8 +126,7 @@ class PLATFORM_EXPORT VideoCaptureImpl
 
   void OnAllClientsFinishedConsumingFrame(
       int buffer_id,
-      scoped_refptr<BufferContext> buffer_context,
-      media::VideoFrameFeedback feedback);
+      scoped_refptr<BufferContext> buffer_context);
 
   void StopDevice();
   void RestartCapture();
@@ -150,7 +150,6 @@ class PLATFORM_EXPORT VideoCaptureImpl
   // RESOURCE_UTILIZATION value from the |metadata| and then runs the given
   // callback, to trampoline back to the IO thread with the values.
   static void DidFinishConsumingFrame(
-      const media::VideoFrameFeedback* feedback,
       BufferFinishedCallback callback_to_io_thread);
 
   void OnStartTimedout();
@@ -204,6 +203,10 @@ class PLATFORM_EXPORT VideoCaptureImpl
 
   std::unique_ptr<gpu::GpuMemoryBufferSupport> gpu_memory_buffer_support_;
 
+  // Stores feedback from the clients, received in |ProcessFeedback()|.
+  // Only accessed on the IO thread.
+  media::VideoFrameFeedback feedback_;
+
   THREAD_CHECKER(io_thread_checker_);
 
   base::OneShotTimer startup_timeout_;

third_party/blink/renderer/platform/wtf/cross_thread_copier.h

@@ -66,6 +66,10 @@ namespace gpu {
 struct SyncToken;
 }
 
+namespace media {
+struct VideoFrameFeedback;
+}
+
 namespace mojo {
 template <typename Interface>
 class PendingReceiver;
@@ -340,6 +344,12 @@ struct CrossThreadCopier<gfx::Size>
   STATIC_ONLY(CrossThreadCopier);
 };
 
+template <>
+struct CrossThreadCopier<media::VideoFrameFeedback>
+    : public CrossThreadCopierPassThrough<media::VideoFrameFeedback> {
+  STATIC_ONLY(CrossThreadCopier);
+};
+
 }  // namespace WTF
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_WTF_CROSS_THREAD_COPIER_H_