admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[WebRtcVideoReceiveStream] Construct flexfec stream on demand.

Browse Source 
...for payload type changes and avoid recreating the main video stream.

Bug: webrtc:11993
Change-Id: I03e44ff25d88caeb082a2e44b2e802d3b9392feb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269244
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37666}
This commit is contained in:
Tommi 2022-08-02 14:37:53 +02:00 committed by WebRTC LUCI CQ
parent 72bc2e24cc
commit dfc4d81fd3
3 changed files with 19 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
media/engine/webrtc_video_engine.cc
View File

@ -3044,8 +3044,7 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetFeedbackParameters(
}
void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetFlexFecPayload(
int payload_type,
bool& flexfec_needs_recreation) {
int payload_type) {
// TODO(bugs.webrtc.org/11993, tommi): See if it is better to always have a
// flexfec stream object around and instead of recreating the video stream,
// reconfigure the flexfec object from within the rtp callback (soon to be on
@ -3067,9 +3066,8 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetFlexFecPayload(
} else if (payload_type != -1) {
flexfec_config_.payload_type = payload_type;
if (flexfec_config_.IsCompleteAndEnabled()) {
// TODO(tommi): Construct new `flexfec_stream_` configure `stream_`
// without having to recreate `stream_`.
flexfec_needs_recreation = true;
flexfec_stream_ = call_->CreateFlexfecReceiveStream(flexfec_config_);
stream_->SetFlexFecProtection(flexfec_stream_);
}
} else {
// Noop. No flexfec stream exists and "new" payload_type == -1.
@ -3082,7 +3080,6 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetRecvParameters(
const ChangedRecvParameters& params) {
RTC_DCHECK(stream_);
bool video_needs_recreation = false;
bool flexfec_needs_recreation = false;
if (params.codec_settings) {
video_needs_recreation = ConfigureCodecs(*params.codec_settings);
}
@ -3102,12 +3099,9 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetRecvParameters(
}
if (params.flexfec_payload_type)
SetFlexFecPayload(*params.flexfec_payload_type, flexfec_needs_recreation);
SetFlexFecPayload(*params.flexfec_payload_type);
// TODO(tommi): When `flexfec_needs_recreation` is `true` and
// `video_needs_recreation` is `false`, recreate only the flexfec stream and
// reconfigure the existing `stream_`.
if (video_needs_recreation || flexfec_needs_recreation) {
if (video_needs_recreation) {
RecreateReceiveStream();
} else {
RTC_DLOG_F(LS_INFO) << "No receive stream recreate needed.";

9
media/engine/webrtc_video_engine.h
View File

@ -505,11 +505,10 @@ class WebRtcVideoChannel : public VideoMediaChannel,
void SetLocalSsrc(uint32_t local_ssrc);
private:
// Attempts to reconfigure an already existing `flexfec_stream_` or sets
// `flexfec_needs_recreation` to `true` if a new instance needs to be
// created by calling `RecreateReceiveStream()`. In all cases
// `SetFlexFecPayload()` will update `flexfec_config_`.
void SetFlexFecPayload(int payload_type, bool& flexfec_needs_recreation);
// Attempts to reconfigure an already existing `flexfec_stream_`, create
// one if the configuration is now complete or remove a flexfec stream
// when disabled.
void SetFlexFecPayload(int payload_type);
void RecreateReceiveStream();

19
media/engine/webrtc_video_engine_unittest.cc
View File

@ -4191,10 +4191,11 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest, SetDefaultRecvCodecsWithSsrc) {
}
// Test changing the configuration after a video stream has been created and
// turn on flexfec. This will result in the video stream being recreated because
// the flexfec stream pointer is injected to the video stream at construction.
// turn on flexfec. This will result in video stream being reconfigured but not
// recreated because the flexfec stream pointer will be given to the already
// existing video stream instance.
TEST_F(WebRtcVideoChannelFlexfecRecvTest,
EnablingFlexfecRecreatesVideoReceiveStream) {
EnablingFlexfecDoesNotRecreateVideoReceiveStream) {
cricket::VideoRecvParameters recv_parameters;
recv_parameters.codecs.push_back(GetEngineCodec("VP8"));
ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters));
@ -4215,13 +4216,13 @@ TEST_F(WebRtcVideoChannelFlexfecRecvTest,
recv_parameters.codecs.push_back(GetEngineCodec("flexfec-03"));
ASSERT_TRUE(channel_->SetRecvParameters(recv_parameters));
// Now the count of created streams will be 3 since the video stream was
// recreated and a flexfec stream was created.
EXPECT_EQ(3, fake_call_->GetNumCreatedReceiveStreams())
<< "Enabling FlexFEC should create FlexfecReceiveStream.";
// The count of created streams will remain 2 despite the creation of a new
// flexfec stream. The existing receive stream will have been reconfigured
// to use the new flexfec instance.
EXPECT_EQ(2, fake_call_->GetNumCreatedReceiveStreams())
<< "Enabling FlexFEC should not create VideoReceiveStreamInterface (1).";
EXPECT_EQ(1U, fake_call_->GetVideoReceiveStreams().size())
<< "Enabling FlexFEC should not create VideoReceiveStreamInterface.";
<< "Enabling FlexFEC should not create VideoReceiveStreamInterface (2).";
EXPECT_EQ(1U, fake_call_->GetFlexfecReceiveStreams().size())
<< "Enabling FlexFEC should create a single FlexfecReceiveStream.";
video_stream = video_streams.front();