From a1b4eb2196b2c2292230faa9afc8c7b8388e936e Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Fri, 4 Nov 2022 14:45:23 +0100 Subject: [PATCH] generateKeyFrame: add rids argument and do the resolution of rids to layers. This has no effect yet since the simulcast encoder adapter (SimulcastEncoderAdapter::Encode), the VP8 encoder (LibvpxVp8Encoder::Encode) and the OpenH264 encoder (H264EncoderImpl::Encode) all generate a key frame for all layers whenever a key frame is requested on one layer. BUG=chromium:1354101 Change-Id: I13f5f1bf136839a68942b0f6bf4f2d5890415250 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280945 Commit-Queue: Ilya Nikolaevskiy Commit-Queue: Philipp Hancke Reviewed-by: Ilya Nikolaevskiy Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#38565} --- api/rtp_sender_interface.h | 4 +++- call/video_send_stream.h | 2 +- media/base/fake_media_engine.cc | 4 +++- media/base/fake_media_engine.h | 3 ++- media/base/media_channel.h | 3 ++- media/engine/fake_webrtc_call.h | 2 +- media/engine/webrtc_video_engine.cc | 11 +++++++---- media/engine/webrtc_video_engine.h | 5 +++-- pc/rtp_sender.cc | 13 +++++++++---- pc/rtp_sender.h | 4 ++-- pc/rtp_sender_proxy.h | 2 +- pc/video_rtp_receiver_unittest.cc | 5 ++++- video/encoder_rtcp_feedback_unittest.cc | 8 +++++--- video/test/mock_video_stream_encoder.h | 5 ++++- video/video_send_stream.cc | 19 +++++++++++++++++-- video/video_send_stream.h | 3 ++- video/video_stream_encoder.cc | 17 ++++++++++++----- video/video_stream_encoder.h | 2 +- video/video_stream_encoder_interface.h | 6 ++++-- 19 files changed, 83 insertions(+), 35 deletions(-) diff --git a/api/rtp_sender_interface.h b/api/rtp_sender_interface.h index 6fc658f9ee..7e84cd420a 100644 --- a/api/rtp_sender_interface.h +++ b/api/rtp_sender_interface.h @@ -105,7 +105,9 @@ class RTC_EXPORT RtpSenderInterface : public rtc::RefCountInterface { encoder_selector) = 0; // TODO(crbug.com/1354101): make pure virtual again after Chrome roll. - virtual RTCError GenerateKeyFrame() { return RTCError::OK(); } + virtual RTCError GenerateKeyFrame(const std::vector& rids) { + return RTCError::OK(); + } protected: ~RtpSenderInterface() override = default; diff --git a/call/video_send_stream.h b/call/video_send_stream.h index 4946253ca8..5fd0bebe20 100644 --- a/call/video_send_stream.h +++ b/call/video_send_stream.h @@ -253,7 +253,7 @@ class VideoSendStream { virtual Stats GetStats() = 0; - virtual void GenerateKeyFrame() = 0; + virtual void GenerateKeyFrame(const std::vector& rids) = 0; protected: virtual ~VideoSendStream() {} diff --git a/media/base/fake_media_engine.cc b/media/base/fake_media_engine.cc index 60f158eb2c..383939a3b5 100644 --- a/media/base/fake_media_engine.cc +++ b/media/base/fake_media_engine.cc @@ -428,7 +428,9 @@ void FakeVideoMediaChannel::ClearRecordableEncodedFrameCallback(uint32_t ssrc) { } void FakeVideoMediaChannel::RequestRecvKeyFrame(uint32_t ssrc) {} -void FakeVideoMediaChannel::GenerateSendKeyFrame(uint32_t ssrc) {} +void FakeVideoMediaChannel::GenerateSendKeyFrame( + uint32_t ssrc, + const std::vector& rids) {} FakeVoiceEngine::FakeVoiceEngine() : fail_create_channel_(false) { // Add a fake audio codec. Note that the name must not be "" as there are diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index b98a2da950..ece77e5174 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -463,7 +463,8 @@ class FakeVideoMediaChannel : public RtpHelper { override; void ClearRecordableEncodedFrameCallback(uint32_t ssrc) override; void RequestRecvKeyFrame(uint32_t ssrc) override; - void GenerateSendKeyFrame(uint32_t ssrc) override; + void GenerateSendKeyFrame(uint32_t ssrc, + const std::vector& rids) override; private: bool SetRecvCodecs(const std::vector& codecs); diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 721bbd3948..5f1d545503 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -926,7 +926,8 @@ class VideoMediaChannel : public MediaChannel, public Delayable { // RTCP feedback. virtual void RequestRecvKeyFrame(uint32_t ssrc) = 0; // Cause generation of a keyframe for `ssrc` on a sending channel. - virtual void GenerateSendKeyFrame(uint32_t ssrc) = 0; + virtual void GenerateSendKeyFrame(uint32_t ssrc, + const std::vector& rids) = 0; virtual std::vector GetSources(uint32_t ssrc) const = 0; }; diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index 65ee0d5b17..1e0568e46e 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -194,7 +194,7 @@ class FakeVideoSendStream final rtc::VideoSourceInterface* source() const { return source_; } - void GenerateKeyFrame() override {} + void GenerateKeyFrame(const std::vector& rids) override {} private: // rtc::VideoSinkInterface implementation. diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 419cf95d9e..5ab7a015d0 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -2828,10 +2828,11 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::RecreateWebRtcStream() { } } -void WebRtcVideoChannel::WebRtcVideoSendStream::GenerateKeyFrame() { +void WebRtcVideoChannel::WebRtcVideoSendStream::GenerateKeyFrame( + const std::vector& rids) { RTC_DCHECK_RUN_ON(&thread_checker_); if (stream_ != NULL) { - stream_->GenerateKeyFrame(); + stream_->GenerateKeyFrame(rids); } else { RTC_LOG(LS_WARNING) << "Absent send stream; ignoring request to generate keyframe."; @@ -3572,11 +3573,13 @@ void WebRtcVideoChannel::RequestRecvKeyFrame(uint32_t ssrc) { } } -void WebRtcVideoChannel::GenerateSendKeyFrame(uint32_t ssrc) { +void WebRtcVideoChannel::GenerateSendKeyFrame( + uint32_t ssrc, + const std::vector& rids) { RTC_DCHECK_RUN_ON(&thread_checker_); auto it = send_streams_.find(ssrc); if (it != send_streams_.end()) { - it->second->GenerateKeyFrame(); + it->second->GenerateKeyFrame(rids); } else { RTC_LOG(LS_ERROR) << "Absent send stream; ignoring key frame generation for ssrc " diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index a0150a8589..ee5b8c3b5a 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -249,7 +249,8 @@ class WebRtcVideoChannel : public VideoMediaChannel, override; void ClearRecordableEncodedFrameCallback(uint32_t ssrc) override; void RequestRecvKeyFrame(uint32_t ssrc) override; - void GenerateSendKeyFrame(uint32_t ssrc) override; + void GenerateSendKeyFrame(uint32_t ssrc, + const std::vector& rids) override; void SetEncoderToPacketizerFrameTransformer( uint32_t ssrc, @@ -391,7 +392,7 @@ class WebRtcVideoChannel : public VideoMediaChannel, void SetEncoderToPacketizerFrameTransformer( rtc::scoped_refptr frame_transformer); - void GenerateKeyFrame(); + void GenerateKeyFrame(const std::vector& rids); private: // Parameters needed to reconstruct the underlying stream. diff --git a/pc/rtp_sender.cc b/pc/rtp_sender.cc index 98e86b3a51..43744ab73e 100644 --- a/pc/rtp_sender.cc +++ b/pc/rtp_sender.cc @@ -596,7 +596,8 @@ rtc::scoped_refptr AudioRtpSender::GetDtmfSender() const { return dtmf_sender_proxy_; } -RTCError AudioRtpSender::GenerateKeyFrame() { +RTCError AudioRtpSender::GenerateKeyFrame( + const std::vector& rids) { RTC_DCHECK_RUN_ON(signaling_thread_); RTC_DLOG(LS_ERROR) << "Tried to get generate a key frame for audio."; return RTCError(RTCErrorType::UNSUPPORTED_OPERATION, @@ -693,11 +694,15 @@ rtc::scoped_refptr VideoRtpSender::GetDtmfSender() const { return nullptr; } -RTCError VideoRtpSender::GenerateKeyFrame() { +RTCError VideoRtpSender::GenerateKeyFrame( + const std::vector& rids) { RTC_DCHECK_RUN_ON(signaling_thread_); + // TODO(crbug.com/1354101): check that rids are a subset of this senders rids + // (or empty). if (video_media_channel() && ssrc_ && !stopped_) { - worker_thread_->PostTask( - [&] { video_media_channel()->GenerateSendKeyFrame(ssrc_); }); + worker_thread_->PostTask([&, rids] { + video_media_channel()->GenerateSendKeyFrame(ssrc_, rids); + }); } else { RTC_LOG(LS_WARNING) << "Tried to generate key frame for sender that is " "stopped or has no media channel."; diff --git a/pc/rtp_sender.h b/pc/rtp_sender.h index 2cfa08dc07..70a9c947aa 100644 --- a/pc/rtp_sender.h +++ b/pc/rtp_sender.h @@ -351,7 +351,7 @@ class AudioRtpSender : public DtmfProviderInterface, public RtpSenderBase { } rtc::scoped_refptr GetDtmfSender() const override; - RTCError GenerateKeyFrame() override; + RTCError GenerateKeyFrame(const std::vector& rids) override; protected: AudioRtpSender(rtc::Thread* worker_thread, @@ -411,7 +411,7 @@ class VideoRtpSender : public RtpSenderBase { } rtc::scoped_refptr GetDtmfSender() const override; - RTCError GenerateKeyFrame() override; + RTCError GenerateKeyFrame(const std::vector& rids) override; RTCError CheckSVCParameters(const RtpParameters& parameters) override; diff --git a/pc/rtp_sender_proxy.h b/pc/rtp_sender_proxy.h index 376fd29d24..a38c8af715 100644 --- a/pc/rtp_sender_proxy.h +++ b/pc/rtp_sender_proxy.h @@ -48,7 +48,7 @@ PROXY_METHOD1(void, PROXY_METHOD1(void, SetEncoderSelector, std::unique_ptr) -PROXY_METHOD0(RTCError, GenerateKeyFrame) +PROXY_METHOD1(RTCError, GenerateKeyFrame, const std::vector&) END_PROXY_MAP(RtpSender) } // namespace webrtc diff --git a/pc/video_rtp_receiver_unittest.cc b/pc/video_rtp_receiver_unittest.cc index c2664590ff..3ec9a28295 100644 --- a/pc/video_rtp_receiver_unittest.cc +++ b/pc/video_rtp_receiver_unittest.cc @@ -50,7 +50,10 @@ class VideoRtpReceiverTest : public testing::Test { (uint32_t), (override)); MOCK_METHOD(void, RequestRecvKeyFrame, (uint32_t), (override)); - MOCK_METHOD(void, GenerateSendKeyFrame, (uint32_t), (override)); + MOCK_METHOD(void, + GenerateSendKeyFrame, + (uint32_t, const std::vector&), + (override)); }; class MockVideoSink : public rtc::VideoSinkInterface { diff --git a/video/encoder_rtcp_feedback_unittest.cc b/video/encoder_rtcp_feedback_unittest.cc index 4cbb747e51..f1ac65d48f 100644 --- a/video/encoder_rtcp_feedback_unittest.cc +++ b/video/encoder_rtcp_feedback_unittest.cc @@ -16,6 +16,8 @@ #include "test/gtest.h" #include "video/test/mock_video_stream_encoder.h" +using ::testing::_; + namespace webrtc { class VieKeyRequestTest : public ::testing::Test { @@ -38,18 +40,18 @@ class VieKeyRequestTest : public ::testing::Test { }; TEST_F(VieKeyRequestTest, CreateAndTriggerRequests) { - EXPECT_CALL(encoder_, SendKeyFrame()).Times(1); + EXPECT_CALL(encoder_, SendKeyFrame(_)).Times(1); encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc); } TEST_F(VieKeyRequestTest, TooManyOnReceivedIntraFrameRequest) { - EXPECT_CALL(encoder_, SendKeyFrame()).Times(1); + EXPECT_CALL(encoder_, SendKeyFrame(_)).Times(1); encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc); encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc); simulated_clock_.AdvanceTimeMilliseconds(10); encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc); - EXPECT_CALL(encoder_, SendKeyFrame()).Times(1); + EXPECT_CALL(encoder_, SendKeyFrame(_)).Times(1); simulated_clock_.AdvanceTimeMilliseconds(300); encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc); encoder_rtcp_feedback_.OnReceivedIntraFrameRequest(kSsrc); diff --git a/video/test/mock_video_stream_encoder.h b/video/test/mock_video_stream_encoder.h index ff246df253..2f982158f0 100644 --- a/video/test/mock_video_stream_encoder.h +++ b/video/test/mock_video_stream_encoder.h @@ -34,7 +34,10 @@ class MockVideoStreamEncoder : public VideoStreamEncoderInterface { (override)); MOCK_METHOD(void, SetSink, (EncoderSink*, bool), (override)); MOCK_METHOD(void, SetStartBitrate, (int), (override)); - MOCK_METHOD(void, SendKeyFrame, (), (override)); + MOCK_METHOD(void, + SendKeyFrame, + (const std::vector&), + (override)); MOCK_METHOD(void, OnLossNotification, (const VideoEncoder::LossNotification&), diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index f245332753..bf5f99aca5 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -343,9 +343,24 @@ void VideoSendStream::DeliverRtcp(const uint8_t* packet, size_t length) { send_stream_.DeliverRtcp(packet, length); } -void VideoSendStream::GenerateKeyFrame() { +void VideoSendStream::GenerateKeyFrame(const std::vector& rids) { + // Map rids to layers. If rids is empty, generate a keyframe for all layers. + std::vector next_frames(config_.rtp.ssrcs.size(), + VideoFrameType::kVideoFrameKey); + if (!config_.rtp.rids.empty() && !rids.empty()) { + std::fill(next_frames.begin(), next_frames.end(), + VideoFrameType::kVideoFrameDelta); + for (const auto& rid : rids) { + for (size_t i = 0; i < config_.rtp.rids.size(); i++) { + if (config_.rtp.rids[i] == rid) { + next_frames[i] = VideoFrameType::kVideoFrameKey; + break; + } + } + } + } if (video_stream_encoder_) { - video_stream_encoder_->SendKeyFrame(); + video_stream_encoder_->SendKeyFrame(next_frames); } } diff --git a/video/video_send_stream.h b/video/video_send_stream.h index a7763731b7..4174d2e5c6 100644 --- a/video/video_send_stream.h +++ b/video/video_send_stream.h @@ -13,6 +13,7 @@ #include #include +#include #include #include "api/fec_controller.h" @@ -93,7 +94,7 @@ class VideoSendStream : public webrtc::VideoSendStream { void StopPermanentlyAndGetRtpStates(RtpStateMap* rtp_state_map, RtpPayloadStateMap* payload_state_map); - void GenerateKeyFrame() override; + void GenerateKeyFrame(const std::vector& rids) override; private: friend class test::VideoSendStreamPeer; diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 746ee79473..de40ccf348 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1981,9 +1981,10 @@ void VideoStreamEncoder::RequestRefreshFrame() { })); } -void VideoStreamEncoder::SendKeyFrame() { +void VideoStreamEncoder::SendKeyFrame( + const std::vector& layers) { if (!encoder_queue_.IsCurrent()) { - encoder_queue_.PostTask([this] { SendKeyFrame(); }); + encoder_queue_.PostTask([this, layers] { SendKeyFrame(layers); }); return; } RTC_DCHECK_RUN_ON(&encoder_queue_); @@ -1998,9 +1999,15 @@ void VideoStreamEncoder::SendKeyFrame() { return; // Shutting down, or not configured yet. } - // TODO(webrtc:10615): Map keyframe request to spatial layer. - std::fill(next_frame_types_.begin(), next_frame_types_.end(), - VideoFrameType::kVideoFrameKey); + if (!layers.empty()) { + RTC_DCHECK_EQ(layers.size(), next_frame_types_.size()); + for (size_t i = 0; i < layers.size() && i < next_frame_types_.size(); i++) { + next_frame_types_[i] = layers[i]; + } + } else { + std::fill(next_frame_types_.begin(), next_frame_types_.end(), + VideoFrameType::kVideoFrameKey); + } } void VideoStreamEncoder::OnLossNotification( diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 9af2e0bcff..e94c369a19 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -111,7 +111,7 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, // guaranteed that no encoded frames will be delivered to the sink. void Stop() override; - void SendKeyFrame() override; + void SendKeyFrame(const std::vector& layers = {}) override; void OnLossNotification( const VideoEncoder::LossNotification& loss_notification) override; diff --git a/video/video_stream_encoder_interface.h b/video/video_stream_encoder_interface.h index 38f180d121..e716572e68 100644 --- a/video/video_stream_encoder_interface.h +++ b/video/video_stream_encoder_interface.h @@ -97,8 +97,10 @@ class VideoStreamEncoderInterface { // resolution. Should be replaced by a construction time setting. virtual void SetStartBitrate(int start_bitrate_bps) = 0; - // Request a key frame. Used for signalling from the remote receiver. - virtual void SendKeyFrame() = 0; + // Request a key frame. Used for signalling from the remote receiver with + // no arguments and for RTCRtpSender.generateKeyFrame with a list of + // rids/layers. + virtual void SendKeyFrame(const std::vector& layers = {}) = 0; // Inform the encoder that a loss has occurred. virtual void OnLossNotification(