WebRtcVideoChannel creates default stream with dummy SSRC on received RTX packet.

This ensure transport feedback is sent for RTX packets that are received before media payload packets.

Bug: webrtc:14795, webrtc:14817
Change-Id: I6a2579b87c8863e003decb2b2559ef51a852cadb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291119
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39174}
This commit is contained in:
Per K 2023-01-23 14:46:03 +01:00 committed by WebRTC LUCI CQ
parent 05ce032486
commit 438b5b4ca5
3 changed files with 197 additions and 138 deletions

View File

@ -13,6 +13,7 @@
#include <stdio.h>
#include <algorithm>
#include <cstdint>
#include <set>
#include <string>
#include <utility>
@ -20,6 +21,7 @@
#include "absl/algorithm/container.h"
#include "absl/functional/bind_front.h"
#include "absl/strings/match.h"
#include "absl/types/optional.h"
#include "api/media_stream_interface.h"
#include "api/video/video_codec_constants.h"
#include "api/video/video_codec_type.h"
@ -583,58 +585,6 @@ WebRtcVideoChannel::WebRtcVideoSendStream::ConfigureVideoEncoderSettings(
return nullptr;
}
DefaultUnsignalledSsrcHandler::DefaultUnsignalledSsrcHandler()
: default_sink_(nullptr) {}
UnsignalledSsrcHandler::Action DefaultUnsignalledSsrcHandler::OnUnsignalledSsrc(
WebRtcVideoChannel* channel,
uint32_t ssrc,
absl::optional<uint32_t> rtx_ssrc) {
absl::optional<uint32_t> default_recv_ssrc = channel->GetUnsignaledSsrc();
if (default_recv_ssrc) {
RTC_LOG(LS_INFO) << "Destroying old default receive stream for SSRC="
<< ssrc << ".";
channel->RemoveRecvStream(*default_recv_ssrc);
}
StreamParams sp = channel->unsignaled_stream_params();
sp.ssrcs.push_back(ssrc);
if (rtx_ssrc) {
sp.AddFidSsrc(ssrc, *rtx_ssrc);
}
RTC_LOG(LS_INFO) << "Creating default receive stream for SSRC=" << ssrc
<< ".";
if (!channel->AddRecvStream(sp, /*default_stream=*/true)) {
RTC_LOG(LS_WARNING) << "Could not create default receive stream.";
}
// SSRC 0 returns default_recv_base_minimum_delay_ms.
const int unsignaled_ssrc = 0;
int default_recv_base_minimum_delay_ms =
channel->GetBaseMinimumPlayoutDelayMs(unsignaled_ssrc).value_or(0);
// Set base minimum delay if it was set before for the default receive stream.
channel->SetBaseMinimumPlayoutDelayMs(ssrc,
default_recv_base_minimum_delay_ms);
channel->SetSink(ssrc, default_sink_);
return kDeliverPacket;
}
rtc::VideoSinkInterface<webrtc::VideoFrame>*
DefaultUnsignalledSsrcHandler::GetDefaultSink() const {
return default_sink_;
}
void DefaultUnsignalledSsrcHandler::SetDefaultSink(
WebRtcVideoChannel* channel,
rtc::VideoSinkInterface<webrtc::VideoFrame>* sink) {
default_sink_ = sink;
absl::optional<uint32_t> default_recv_ssrc = channel->GetUnsignaledSsrc();
if (default_recv_ssrc) {
channel->SetSink(*default_recv_ssrc, default_sink_);
}
}
WebRtcVideoEngine::WebRtcVideoEngine(
std::unique_ptr<webrtc::VideoEncoderFactory> video_encoder_factory,
std::unique_ptr<webrtc::VideoDecoderFactory> video_decoder_factory,
@ -724,7 +674,7 @@ WebRtcVideoChannel::WebRtcVideoChannel(
: VideoMediaChannel(call->network_thread(), config.enable_dscp),
worker_thread_(call->worker_thread()),
call_(call),
unsignalled_ssrc_handler_(&default_unsignalled_ssrc_handler_),
default_sink_(nullptr),
video_config_(config.video),
encoder_factory_(encoder_factory),
decoder_factory_(decoder_factory),
@ -1151,7 +1101,7 @@ webrtc::RtpParameters WebRtcVideoChannel::GetDefaultRtpReceiveParameters()
const {
RTC_DCHECK_RUN_ON(&thread_checker_);
webrtc::RtpParameters rtp_params;
if (!default_unsignalled_ssrc_handler_.GetDefaultSink()) {
if (!default_sink_) {
// Getting parameters on a default, unsignaled video receive stream but
// because we've not configured to receive such a stream, `encodings` is
// empty.
@ -1648,7 +1598,7 @@ void WebRtcVideoChannel::SetDefaultSink(
rtc::VideoSinkInterface<webrtc::VideoFrame>* sink) {
RTC_DCHECK_RUN_ON(&thread_checker_);
RTC_LOG(LS_INFO) << "SetDefaultSink: " << (sink ? "(ptr)" : "nullptr");
default_unsignalled_ssrc_handler_.SetDefaultSink(this, sink);
default_sink_ = sink;
}
bool WebRtcVideoChannel::GetSendStats(VideoMediaSendInfo* info) {
@ -1809,35 +1759,6 @@ bool WebRtcVideoChannel::MaybeCreateDefaultReceiveStream(
return false;
}
absl::optional<uint32_t> rtx_ssrc;
uint32_t ssrc = packet.Ssrc();
// See if this payload_type is registered as one that usually gets its
// own SSRC (RTX) or at least is safe to drop either way (FEC). If it
// is, and it wasn't handled above by DeliverPacket, that means we don't
// know what stream it associates with, and we shouldn't ever create an
// implicit channel for these.
for (auto& codec : recv_codecs_) {
if (packet.PayloadType() == codec.ulpfec.red_rtx_payload_type ||
packet.PayloadType() == codec.ulpfec.ulpfec_payload_type) {
return false;
}
if (packet.PayloadType() == codec.rtx_payload_type) {
// As we don't support receiving simulcast there can only be one RTX
// stream, which will be associated with unsignaled media stream.
// It is not possible to update the ssrcs of a receive stream, so we
// recreate it insead if found.
auto default_ssrc = GetUnsignaledSsrc();
if (!default_ssrc) {
return false;
}
rtx_ssrc = ssrc;
ssrc = *default_ssrc;
// Allow recreating the receive stream even if the RTX packet is
// received just after the media packet.
last_unsignalled_ssrc_creation_time_ms_.reset();
break;
}
}
if (packet.PayloadType() == recv_flexfec_payload_type_) {
return false;
}
@ -1849,33 +1770,102 @@ bool WebRtcVideoChannel::MaybeCreateDefaultReceiveStream(
if (demuxer_criteria_id_ != demuxer_criteria_completed_id_) {
return false;
}
// Ignore unknown ssrcs if we recently created an unsignalled receive
// stream since this shouldn't happen frequently. Getting into a state
// of creating decoders on every packet eats up processing time (e.g.
// https://crbug.com/1069603) and this cooldown prevents that.
if (last_unsignalled_ssrc_creation_time_ms_.has_value()) {
int64_t now_ms = rtc::TimeMillis();
if (now_ms - last_unsignalled_ssrc_creation_time_ms_.value() <
kUnsignaledSsrcCooldownMs) {
// We've already created an unsignalled ssrc stream within the last
// 0.5 s, ignore with a warning.
RTC_LOG(LS_WARNING)
<< "Another unsignalled ssrc packet arrived shortly after the "
<< "creation of an unsignalled ssrc stream. Dropping packet.";
// See if this payload_type is registered as one that usually gets its
// own SSRC (RTX) or at least is safe to drop either way (FEC). If it
// is, and it wasn't handled above by DeliverPacket, that means we don't
// know what stream it associates with, and we shouldn't ever create an
// implicit channel for these.
bool is_rtx_payload = false;
for (auto& codec : recv_codecs_) {
if (packet.PayloadType() == codec.ulpfec.red_rtx_payload_type ||
packet.PayloadType() == codec.ulpfec.ulpfec_payload_type) {
return false;
}
}
// Let the unsignalled ssrc handler decide whether to drop or deliver.
switch (unsignalled_ssrc_handler_->OnUnsignalledSsrc(this, ssrc, rtx_ssrc)) {
case UnsignalledSsrcHandler::kDropPacket:
return false;
case UnsignalledSsrcHandler::kDeliverPacket:
if (packet.PayloadType() == codec.rtx_payload_type) {
is_rtx_payload = true;
break;
}
}
if (is_rtx_payload) {
// As we don't support receiving simulcast there can only be one RTX
// stream, which will be associated with unsignaled media stream.
absl::optional<uint32_t> current_default_ssrc = GetUnsignaledSsrc();
if (current_default_ssrc) {
// TODO(bug.webrtc.org/14817): Consider associating the existing default
// stream with this RTX stream instead of recreating.
ReCreateDefaulReceiveStream(/*ssrc =*/*current_default_ssrc,
packet.Ssrc());
} else {
// Received unsignaled RTX packet before a media packet. Create a default
// stream with a "random" SSRC and the RTX SSRC from the packet. The
// stream will be recreated on the first media packet, unless we are
// extremely lucky and used the right media SSRC.
ReCreateDefaulReceiveStream(/*ssrc =*/14795, /*rtx_ssrc=*/packet.Ssrc());
}
return true;
} else {
// Ignore unknown ssrcs if we recently created an unsignalled receive
// stream since this shouldn't happen frequently. Getting into a state
// of creating decoders on every packet eats up processing time (e.g.
// https://crbug.com/1069603) and this cooldown prevents that.
if (last_unsignalled_ssrc_creation_time_ms_.has_value()) {
int64_t now_ms = rtc::TimeMillis();
if (now_ms - last_unsignalled_ssrc_creation_time_ms_.value() <
kUnsignaledSsrcCooldownMs) {
// We've already created an unsignalled ssrc stream within the last
// 0.5 s, ignore with a warning.
RTC_LOG(LS_WARNING)
<< "Another unsignalled ssrc packet arrived shortly after the "
<< "creation of an unsignalled ssrc stream. Dropping packet.";
return false;
}
}
}
// TODO(bug.webrtc.org/14817): Consider creating a default stream with a fake
// RTX ssrc that can be updated when the real SSRC is known if rtx has been
// negotiated.
ReCreateDefaulReceiveStream(packet.Ssrc(), absl::nullopt);
last_unsignalled_ssrc_creation_time_ms_ = rtc::TimeMillis();
return true;
}
void WebRtcVideoChannel::ReCreateDefaulReceiveStream(
uint32_t ssrc,
absl::optional<uint32_t> rtx_ssrc) {
RTC_DCHECK_RUN_ON(&thread_checker_);
absl::optional<uint32_t> default_recv_ssrc = GetUnsignaledSsrc();
if (default_recv_ssrc) {
RTC_LOG(LS_INFO) << "Destroying old default receive stream for SSRC="
<< ssrc << ".";
RemoveRecvStream(*default_recv_ssrc);
}
StreamParams sp = unsignaled_stream_params();
sp.ssrcs.push_back(ssrc);
if (rtx_ssrc) {
sp.AddFidSsrc(ssrc, *rtx_ssrc);
}
RTC_LOG(LS_INFO) << "Creating default receive stream for SSRC=" << ssrc
<< ".";
if (!AddRecvStream(sp, /*default_stream=*/true)) {
RTC_LOG(LS_WARNING) << "Could not create default receive stream.";
}
// SSRC 0 returns default_recv_base_minimum_delay_ms.
const int unsignaled_ssrc = 0;
int default_recv_base_minimum_delay_ms =
GetBaseMinimumPlayoutDelayMs(unsignaled_ssrc).value_or(0);
// Set base minimum delay if it was set before for the default receive
// stream.
SetBaseMinimumPlayoutDelayMs(ssrc, default_recv_base_minimum_delay_ms);
SetSink(ssrc, default_sink_);
}
void WebRtcVideoChannel::OnPacketSent(const rtc::SentPacket& sent_packet) {
RTC_DCHECK_RUN_ON(&network_thread_checker_);
// TODO(tommi): We shouldn't need to go through call_ to deliver this

View File

@ -60,36 +60,6 @@ std::map<uint32_t, webrtc::VideoSendStream::StreamStats>
MergeInfoAboutOutboundRtpSubstreamsForTesting(
const std::map<uint32_t, webrtc::VideoSendStream::StreamStats>& substreams);
class UnsignalledSsrcHandler {
public:
enum Action {
kDropPacket,
kDeliverPacket,
};
virtual Action OnUnsignalledSsrc(WebRtcVideoChannel* channel,
uint32_t ssrc,
absl::optional<uint32_t> rtx_ssrc) = 0;
virtual ~UnsignalledSsrcHandler() = default;
};
// TODO(pbos): Remove, use external handlers only.
class DefaultUnsignalledSsrcHandler : public UnsignalledSsrcHandler {
public:
DefaultUnsignalledSsrcHandler();
Action OnUnsignalledSsrc(WebRtcVideoChannel* channel,
uint32_t ssrc,
absl::optional<uint32_t> rtx_ssrc) override;
rtc::VideoSinkInterface<webrtc::VideoFrame>* GetDefaultSink() const;
void SetDefaultSink(WebRtcVideoChannel* channel,
rtc::VideoSinkInterface<webrtc::VideoFrame>* sink);
virtual ~DefaultUnsignalledSsrcHandler() = default;
private:
rtc::VideoSinkInterface<webrtc::VideoFrame>* default_sink_;
};
// WebRtcVideoEngine is used for the new native WebRTC Video API (webrtc:1667).
class WebRtcVideoEngine : public VideoEngineInterface {
public:
@ -321,6 +291,8 @@ class WebRtcVideoChannel : public VideoMediaChannel,
bool MaybeCreateDefaultReceiveStream(
const webrtc::RtpPacketReceived& parsed_packet)
RTC_EXCLUSIVE_LOCKS_REQUIRED(thread_checker_);
void ReCreateDefaulReceiveStream(uint32_t ssrc,
absl::optional<uint32_t> rtx_ssrc);
void ConfigureReceiverRtp(
webrtc::VideoReceiveStreamInterface::Config* config,
webrtc::FlexfecReceiveStream::Config* flexfec_config,
@ -600,9 +572,7 @@ class WebRtcVideoChannel : public VideoMediaChannel,
bool sending_ RTC_GUARDED_BY(thread_checker_);
webrtc::Call* const call_;
DefaultUnsignalledSsrcHandler default_unsignalled_ssrc_handler_
RTC_GUARDED_BY(thread_checker_);
UnsignalledSsrcHandler* const unsignalled_ssrc_handler_
rtc::VideoSinkInterface<webrtc::VideoFrame>* default_sink_
RTC_GUARDED_BY(thread_checker_);
// Delay for unsignaled streams, which may be set before the stream exists.

View File

@ -53,6 +53,8 @@
#include "media/engine/fake_webrtc_call.h"
#include "media/engine/fake_webrtc_video_engine.h"
#include "media/engine/webrtc_voice_engine.h"
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
#include "modules/rtp_rtcp/source/rtcp_packet.h"
#include "modules/rtp_rtcp/source/rtp_packet.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/video_coding/svc/scalability_mode_util.h"
@ -66,6 +68,7 @@
#include "test/fake_decoder.h"
#include "test/frame_forwarder.h"
#include "test/gmock.h"
#include "test/rtcp_packet_parser.h"
#include "test/scoped_key_value_config.h"
#include "test/time_controller/simulated_time_controller.h"
#include "video/config/simulcast.h"
@ -85,12 +88,14 @@ using ::testing::Return;
using ::testing::SizeIs;
using ::testing::StrNe;
using ::testing::Values;
using ::testing::WithArg;
using ::webrtc::BitrateConstraints;
using ::webrtc::kDefaultScalabilityModeStr;
using ::webrtc::RtpExtension;
using ::webrtc::RtpPacket;
using ::webrtc::RtpPacketReceived;
using ::webrtc::ScalabilityMode;
using ::webrtc::test::RtcpPacketParser;
namespace {
static const int kDefaultQpMax = 56;
@ -246,6 +251,24 @@ class MockVideoSource : public rtc::VideoSourceInterface<webrtc::VideoFrame> {
(override));
};
class MockNetworkInterface : public cricket::MediaChannelNetworkInterface {
public:
MOCK_METHOD(bool,
SendPacket,
(rtc::CopyOnWriteBuffer * packet,
const rtc::PacketOptions& options),
(override));
MOCK_METHOD(bool,
SendRtcp,
(rtc::CopyOnWriteBuffer * packet,
const rtc::PacketOptions& options),
(override));
MOCK_METHOD(int,
SetOption,
(SocketType type, rtc::Socket::Option opt, int option),
(override));
};
std::vector<webrtc::Resolution> GetStreamResolutions(
const std::vector<webrtc::VideoStream>& streams) {
std::vector<webrtc::Resolution> res;
@ -832,6 +855,51 @@ void WebRtcVideoEngineTest::ExpectRtpCapabilitySupport(const char* uri,
}
}
TEST_F(WebRtcVideoEngineTest, SendsFeedbackAfterUnsignaledRtxPacket) {
// Setup a channel with VP8, RTX and transport sequence number header
// extension. Receive stream is not explicitly configured.
AddSupportedVideoCodecType("VP8");
std::vector<VideoCodec> supported_codecs =
engine_.recv_codecs(/*include_rtx=*/true);
ASSERT_EQ(supported_codecs[1].name, "rtx");
int rtx_payload_type = supported_codecs[1].id;
MockNetworkInterface network;
RtcpPacketParser rtcp_parser;
ON_CALL(network, SendRtcp)
.WillByDefault(testing::DoAll(
WithArg<0>([&](rtc::CopyOnWriteBuffer* packet) {
ASSERT_TRUE(rtcp_parser.Parse(packet->cdata(), packet->size()));
}),
Return(true)));
std::unique_ptr<VideoMediaChannel> channel(engine_.CreateMediaChannel(
call_.get(), GetMediaConfig(), VideoOptions(), webrtc::CryptoOptions(),
video_bitrate_allocator_factory_.get()));
cricket::VideoRecvParameters parameters;
parameters.codecs = supported_codecs;
const int kTransportSeqExtensionId = 1;
parameters.extensions.push_back(RtpExtension(
RtpExtension::kTransportSequenceNumberUri, kTransportSeqExtensionId));
ASSERT_TRUE(channel->SetRecvParameters(parameters));
channel->SetInterface(&network);
channel->AsVideoReceiveChannel()->OnReadyToSend(true);
// Inject a RTX packet.
webrtc::RtpHeaderExtensionMap extension_map(parameters.extensions);
webrtc::RtpPacketReceived packet(&extension_map);
packet.SetMarker(true);
packet.SetPayloadType(rtx_payload_type);
packet.SetSsrc(999);
packet.SetExtension<webrtc::TransportSequenceNumber>(7);
uint8_t* buf_ptr = packet.AllocatePayload(11);
memset(buf_ptr, 0, 11); // Pass MSAN (don't care about bytes 1-9)
channel->AsVideoReceiveChannel()->OnPacketReceived(packet);
// Expect that feedback is sent after a while.
time_controller_.AdvanceTime(webrtc::TimeDelta::Seconds(1));
EXPECT_GT(rtcp_parser.transport_feedback()->num_packets(), 0);
channel->SetInterface(nullptr);
}
TEST_F(WebRtcVideoEngineTest, UsesSimulcastAdapterForVp8Factories) {
AddSupportedVideoCodecType("VP8");
@ -7178,12 +7246,12 @@ TEST_F(WebRtcVideoChannelTest, Vp9PacketCreatesUnsignalledStream) {
true /* expect_created_receive_stream */);
}
TEST_F(WebRtcVideoChannelTest, RtxPacketDoesntCreateUnsignalledStream) {
TEST_F(WebRtcVideoChannelTest, RtxPacketCreateUnsignalledStream) {
AssignDefaultAptRtxTypes();
const cricket::VideoCodec vp8 = GetEngineCodec("VP8");
const int rtx_vp8_payload_type = default_apt_rtx_types_[vp8.id];
TestReceiveUnsignaledSsrcPacket(rtx_vp8_payload_type,
false /* expect_created_receive_stream */);
true /* expect_created_receive_stream */);
}
TEST_F(WebRtcVideoChannelTest, UlpfecPacketDoesntCreateUnsignalledStream) {
@ -7237,6 +7305,37 @@ TEST_F(WebRtcVideoChannelTest,
<< "Receive stream should have correct rtx ssrc";
}
TEST_F(WebRtcVideoChannelTest,
MediaPacketAfterRtxImmediatelyRecreatesUnsignalledStream) {
AssignDefaultAptRtxTypes();
const cricket::VideoCodec vp8 = GetEngineCodec("VP8");
const int payload_type = vp8.id;
const int rtx_vp8_payload_type = default_apt_rtx_types_[vp8.id];
const uint32_t ssrc = kIncomingUnsignalledSsrc;
const uint32_t rtx_ssrc = ssrc + 1;
// Send rtx packet.
RtpPacketReceived rtx_packet;
rtx_packet.SetPayloadType(rtx_vp8_payload_type);
rtx_packet.SetSsrc(rtx_ssrc);
receive_channel_->OnPacketReceived(rtx_packet);
rtc::Thread::Current()->ProcessMessages(0);
EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
// Send media packet.
RtpPacketReceived packet;
packet.SetPayloadType(payload_type);
packet.SetSsrc(ssrc);
ReceivePacketAndAdvanceTime(packet);
EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
// Check receive stream has been recreated with correct ssrcs.
auto recv_stream = fake_call_->GetVideoReceiveStreams().front();
auto& config = recv_stream->GetConfig();
EXPECT_EQ(config.rtp.remote_ssrc, ssrc)
<< "Receive stream should have correct media ssrc";
}
// Test that receiving any unsignalled SSRC works even if it changes.
// The first unsignalled SSRC received will create a default receive stream.
// Any different unsignalled SSRC received will replace the default.