From 3dd73ae6f4d0e1fa39145677341712ebfd65ac44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Mon, 16 Jan 2023 10:41:28 +0100 Subject: [PATCH] Surface the SetMetadata() method so that Chromium can use it. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RTPVideoHeader is changed to non-const to allow modifying it. We want to do this when implementing setMetadata() in JavaScript or when refactoring clone() as "construct + set bytes + setMetadata". Unblocks https://chromium-review.googlesource.com/c/chromium/src/+/4164979. Bug: webrtc:14709 Change-Id: I6089df9c03e9aa33feeb0830dd240dd456cb565e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290981 Reviewed-by: Harald Alvestrand Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/main@{#39113} --- api/frame_transformer_interface.h | 3 +++ api/test/mock_transformable_video_frame.h | 4 ++++ .../source/frame_transformer_factory_unittest.cc | 4 ++++ .../rtp_sender_video_frame_transformer_delegate.cc | 14 ++++++++++++-- ...o_stream_receiver_frame_transformer_delegate.cc | 5 +++++ 5 files changed, 28 insertions(+), 2 deletions(-) diff --git a/api/frame_transformer_interface.h b/api/frame_transformer_interface.h index 5efd3ea051..fba09d5ff0 100644 --- a/api/frame_transformer_interface.h +++ b/api/frame_transformer_interface.h @@ -62,6 +62,9 @@ class TransformableVideoFrameInterface : public TransformableFrameInterface { virtual std::vector GetAdditionalData() const = 0; virtual const VideoFrameMetadata& GetMetadata() const = 0; + // TODO(https://crbug.com/webrtc/14709): Make pure virtual when Chromium MOCK + // has implemented this. + virtual void SetMetadata(const VideoFrameMetadata&) {} }; // Extends the TransformableFrameInterface to expose audio-specific information. diff --git a/api/test/mock_transformable_video_frame.h b/api/test/mock_transformable_video_frame.h index 5cebcaba80..18b3de534e 100644 --- a/api/test/mock_transformable_video_frame.h +++ b/api/test/mock_transformable_video_frame.h @@ -31,6 +31,10 @@ class MockTransformableVideoFrame GetMetadata, (), (const, override)); + MOCK_METHOD(void, + SetMetadata, + (const webrtc::VideoFrameMetadata&), + (override)); }; } // namespace webrtc diff --git a/modules/rtp_rtcp/source/frame_transformer_factory_unittest.cc b/modules/rtp_rtcp/source/frame_transformer_factory_unittest.cc index e011a76ed5..65a0e4cbb4 100644 --- a/modules/rtp_rtcp/source/frame_transformer_factory_unittest.cc +++ b/modules/rtp_rtcp/source/frame_transformer_factory_unittest.cc @@ -48,6 +48,10 @@ class MockTransformableVideoFrame GetMetadata, (), (const, override)); + MOCK_METHOD(void, + SetMetadata, + (const webrtc::VideoFrameMetadata&), + (override)); }; TEST(FrameTransformerFactory, CloneVideoFrame) { diff --git a/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc b/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc index 02194391af..27b6a17a20 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc @@ -69,6 +69,12 @@ class TransformableVideoSenderFrame : public TransformableVideoFrameInterface { } const VideoFrameMetadata& GetMetadata() const override { return metadata_; } + void SetMetadata(const VideoFrameMetadata& metadata) override { + header_.SetFromMetadata(metadata); + // We have to keep a local copy because GetMetadata() has to return a + // reference. + metadata_ = header_.GetAsMetadata(); + } const RTPVideoHeader& GetHeader() const { return header_; } uint8_t GetPayloadType() const override { return payload_type_; } @@ -83,8 +89,12 @@ class TransformableVideoSenderFrame : public TransformableVideoFrameInterface { private: rtc::scoped_refptr encoded_data_; - const RTPVideoHeader header_; - const VideoFrameMetadata metadata_; + RTPVideoHeader header_; + // This is a copy of `header_.GetAsMetadata()`, only needed because the + // interface says GetMetadata() must return a const ref rather than a value. + // TODO(crbug.com/webrtc/14709): Change the interface and delete this variable + // to reduce risk of it getting out-of-sync with `header_.GetAsMetadata()`. + VideoFrameMetadata metadata_; const VideoFrameType frame_type_; const uint8_t payload_type_; const absl::optional codec_type_ = absl::nullopt; diff --git a/video/rtp_video_stream_receiver_frame_transformer_delegate.cc b/video/rtp_video_stream_receiver_frame_transformer_delegate.cc index 16015beee5..b1907fa7a2 100644 --- a/video/rtp_video_stream_receiver_frame_transformer_delegate.cc +++ b/video/rtp_video_stream_receiver_frame_transformer_delegate.cc @@ -15,6 +15,7 @@ #include "absl/memory/memory.h" #include "modules/rtp_rtcp/source/rtp_descriptor_authentication.h" +#include "rtc_base/checks.h" #include "rtc_base/thread.h" namespace webrtc { @@ -53,6 +54,10 @@ class TransformableVideoReceiverFrame } const VideoFrameMetadata& GetMetadata() const override { return metadata_; } + void SetMetadata(const VideoFrameMetadata&) override { + RTC_DCHECK_NOTREACHED() + << "TransformableVideoReceiverFrame::SetMetadata is not implemented"; + } std::unique_ptr ExtractFrame() && { return std::move(frame_);