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 <ilnik@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38565}
diff --git a/api/rtp_sender_interface.h b/api/rtp_sender_interface.h
index 6fc658f..7e84cd4 100644
--- a/api/rtp_sender_interface.h
+++ b/api/rtp_sender_interface.h
@@ -105,7 +105,9 @@
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<std::string>& rids) {
+ return RTCError::OK();
+ }
protected:
~RtpSenderInterface() override = default;
diff --git a/call/video_send_stream.h b/call/video_send_stream.h
index 4946253..5fd0beb 100644
--- a/call/video_send_stream.h
+++ b/call/video_send_stream.h
@@ -253,7 +253,7 @@
virtual Stats GetStats() = 0;
- virtual void GenerateKeyFrame() = 0;
+ virtual void GenerateKeyFrame(const std::vector<std::string>& rids) = 0;
protected:
virtual ~VideoSendStream() {}
diff --git a/media/base/fake_media_engine.cc b/media/base/fake_media_engine.cc
index 60f158e..383939a 100644
--- a/media/base/fake_media_engine.cc
+++ b/media/base/fake_media_engine.cc
@@ -428,7 +428,9 @@
}
void FakeVideoMediaChannel::RequestRecvKeyFrame(uint32_t ssrc) {}
-void FakeVideoMediaChannel::GenerateSendKeyFrame(uint32_t ssrc) {}
+void FakeVideoMediaChannel::GenerateSendKeyFrame(
+ uint32_t ssrc,
+ const std::vector<std::string>& 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 b98a2da..ece77e5 100644
--- a/media/base/fake_media_engine.h
+++ b/media/base/fake_media_engine.h
@@ -463,7 +463,8 @@
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<std::string>& rids) override;
private:
bool SetRecvCodecs(const std::vector<VideoCodec>& codecs);
diff --git a/media/base/media_channel.h b/media/base/media_channel.h
index 721bbd3..5f1d545 100644
--- a/media/base/media_channel.h
+++ b/media/base/media_channel.h
@@ -926,7 +926,8 @@
// 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<std::string>& rids) = 0;
virtual std::vector<webrtc::RtpSource> GetSources(uint32_t ssrc) const = 0;
};
diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h
index 65ee0d5..1e0568e 100644
--- a/media/engine/fake_webrtc_call.h
+++ b/media/engine/fake_webrtc_call.h
@@ -194,7 +194,7 @@
rtc::VideoSourceInterface<webrtc::VideoFrame>* source() const {
return source_;
}
- void GenerateKeyFrame() override {}
+ void GenerateKeyFrame(const std::vector<std::string>& rids) override {}
private:
// rtc::VideoSinkInterface<VideoFrame> implementation.
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 419cf95..5ab7a01 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -2828,10 +2828,11 @@
}
}
-void WebRtcVideoChannel::WebRtcVideoSendStream::GenerateKeyFrame() {
+void WebRtcVideoChannel::WebRtcVideoSendStream::GenerateKeyFrame(
+ const std::vector<std::string>& 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::GenerateSendKeyFrame(uint32_t ssrc) {
+void WebRtcVideoChannel::GenerateSendKeyFrame(
+ uint32_t ssrc,
+ const std::vector<std::string>& 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 a0150a8..ee5b8c3 100644
--- a/media/engine/webrtc_video_engine.h
+++ b/media/engine/webrtc_video_engine.h
@@ -249,7 +249,8 @@
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<std::string>& rids) override;
void SetEncoderToPacketizerFrameTransformer(
uint32_t ssrc,
@@ -391,7 +392,7 @@
void SetEncoderToPacketizerFrameTransformer(
rtc::scoped_refptr<webrtc::FrameTransformerInterface>
frame_transformer);
- void GenerateKeyFrame();
+ void GenerateKeyFrame(const std::vector<std::string>& rids);
private:
// Parameters needed to reconstruct the underlying stream.
diff --git a/pc/rtp_sender.cc b/pc/rtp_sender.cc
index 98e86b3..43744ab 100644
--- a/pc/rtp_sender.cc
+++ b/pc/rtp_sender.cc
@@ -596,7 +596,8 @@
return dtmf_sender_proxy_;
}
-RTCError AudioRtpSender::GenerateKeyFrame() {
+RTCError AudioRtpSender::GenerateKeyFrame(
+ const std::vector<std::string>& 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 @@
return nullptr;
}
-RTCError VideoRtpSender::GenerateKeyFrame() {
+RTCError VideoRtpSender::GenerateKeyFrame(
+ const std::vector<std::string>& 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 2cfa08d..70a9c94 100644
--- a/pc/rtp_sender.h
+++ b/pc/rtp_sender.h
@@ -351,7 +351,7 @@
}
rtc::scoped_refptr<DtmfSenderInterface> GetDtmfSender() const override;
- RTCError GenerateKeyFrame() override;
+ RTCError GenerateKeyFrame(const std::vector<std::string>& rids) override;
protected:
AudioRtpSender(rtc::Thread* worker_thread,
@@ -411,7 +411,7 @@
}
rtc::scoped_refptr<DtmfSenderInterface> GetDtmfSender() const override;
- RTCError GenerateKeyFrame() override;
+ RTCError GenerateKeyFrame(const std::vector<std::string>& rids) override;
RTCError CheckSVCParameters(const RtpParameters& parameters) override;
diff --git a/pc/rtp_sender_proxy.h b/pc/rtp_sender_proxy.h
index 376fd29..a38c8af 100644
--- a/pc/rtp_sender_proxy.h
+++ b/pc/rtp_sender_proxy.h
@@ -48,7 +48,7 @@
PROXY_METHOD1(void,
SetEncoderSelector,
std::unique_ptr<VideoEncoderFactory::EncoderSelectorInterface>)
-PROXY_METHOD0(RTCError, GenerateKeyFrame)
+PROXY_METHOD1(RTCError, GenerateKeyFrame, const std::vector<std::string>&)
END_PROXY_MAP(RtpSender)
} // namespace webrtc
diff --git a/pc/video_rtp_receiver_unittest.cc b/pc/video_rtp_receiver_unittest.cc
index c266459..3ec9a28 100644
--- a/pc/video_rtp_receiver_unittest.cc
+++ b/pc/video_rtp_receiver_unittest.cc
@@ -50,7 +50,10 @@
(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<std::string>&),
+ (override));
};
class MockVideoSink : public rtc::VideoSinkInterface<RecordableEncodedFrame> {
diff --git a/video/encoder_rtcp_feedback_unittest.cc b/video/encoder_rtcp_feedback_unittest.cc
index 4cbb747..f1ac65d 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 @@
};
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 ff246df..2f98215 100644
--- a/video/test/mock_video_stream_encoder.h
+++ b/video/test/mock_video_stream_encoder.h
@@ -34,7 +34,10 @@
(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<VideoFrameType>&),
+ (override));
MOCK_METHOD(void,
OnLossNotification,
(const VideoEncoder::LossNotification&),
diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc
index f245332..bf5f99a 100644
--- a/video/video_send_stream.cc
+++ b/video/video_send_stream.cc
@@ -343,9 +343,24 @@
send_stream_.DeliverRtcp(packet, length);
}
-void VideoSendStream::GenerateKeyFrame() {
+void VideoSendStream::GenerateKeyFrame(const std::vector<std::string>& rids) {
+ // Map rids to layers. If rids is empty, generate a keyframe for all layers.
+ std::vector<VideoFrameType> 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 a776373..4174d2e 100644
--- a/video/video_send_stream.h
+++ b/video/video_send_stream.h
@@ -13,6 +13,7 @@
#include <map>
#include <memory>
+#include <string>
#include <vector>
#include "api/fec_controller.h"
@@ -93,7 +94,7 @@
void StopPermanentlyAndGetRtpStates(RtpStateMap* rtp_state_map,
RtpPayloadStateMap* payload_state_map);
- void GenerateKeyFrame() override;
+ void GenerateKeyFrame(const std::vector<std::string>& rids) override;
private:
friend class test::VideoSendStreamPeer;
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 746ee79..de40ccf 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -1981,9 +1981,10 @@
}));
}
-void VideoStreamEncoder::SendKeyFrame() {
+void VideoStreamEncoder::SendKeyFrame(
+ const std::vector<VideoFrameType>& 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 @@
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 9af2e0b..e94c369 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -111,7 +111,7 @@
// guaranteed that no encoded frames will be delivered to the sink.
void Stop() override;
- void SendKeyFrame() override;
+ void SendKeyFrame(const std::vector<VideoFrameType>& 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 38f180d..e716572 100644
--- a/video/video_stream_encoder_interface.h
+++ b/video/video_stream_encoder_interface.h
@@ -97,8 +97,10 @@
// 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<VideoFrameType>& layers = {}) = 0;
// Inform the encoder that a loss has occurred.
virtual void OnLossNotification(
