Fix RtpReceiver.GetParameters when SSRCs aren't signaled.
When SSRCs aren't signaled, an SSRC of 0 is used internally to mean
"the default receive stream". But this wasn't working with the
implementation of GetRtpReceiveParameters in the audio/video
engines. This was breaking RtpReceiver.GetParameters in this situation,
as well as the new getStats implementation (which relies on
GetParameters).
The new implementation will fail if *no* default receive stream is
configured (meaning no default sink is set), and otherwise will return
a default RtpEncodingParameters (later it will be filled with relevant
SDP parameters as they're implemented).
BUG=webrtc:6971
Review-Url: https://codereview.webrtc.org/2806173002
Cr-Original-Commit-Position: refs/heads/master@{#17803}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 3bc15103aea1dd2fdfc01b833b62f34171c82e81
diff --git a/media/base/mediachannel.h b/media/base/mediachannel.h
index ca02c72..6f14b7a 100644
--- a/media/base/mediachannel.h
+++ b/media/base/mediachannel.h
@@ -973,6 +973,11 @@
virtual bool SetRtpSendParameters(
uint32_t ssrc,
const webrtc::RtpParameters& parameters) = 0;
+ // Get the receive parameters for the incoming stream identified by |ssrc|.
+ // If |ssrc| is 0, retrieve the receive parameters for the default receive
+ // stream, which is used when SSRCs are not signaled. Note that calling with
+ // an |ssrc| of 0 will return encoding parameters with an unset |ssrc|
+ // member.
virtual webrtc::RtpParameters GetRtpReceiveParameters(
uint32_t ssrc) const = 0;
virtual bool SetRtpReceiveParameters(
@@ -1053,6 +1058,11 @@
virtual bool SetRtpSendParameters(
uint32_t ssrc,
const webrtc::RtpParameters& parameters) = 0;
+ // Get the receive parameters for the incoming stream identified by |ssrc|.
+ // If |ssrc| is 0, retrieve the receive parameters for the default receive
+ // stream, which is used when SSRCs are not signaled. Note that calling with
+ // an |ssrc| of 0 will return encoding parameters with an unset |ssrc|
+ // member.
virtual webrtc::RtpParameters GetRtpReceiveParameters(
uint32_t ssrc) const = 0;
virtual bool SetRtpReceiveParameters(
@@ -1070,7 +1080,7 @@
const VideoOptions* options,
rtc::VideoSourceInterface<webrtc::VideoFrame>* source) = 0;
// Sets the sink object to be used for the specified stream.
- // If SSRC is 0, the renderer is used for the 'default' stream.
+ // If SSRC is 0, the sink is used for the 'default' stream.
virtual bool SetSink(uint32_t ssrc,
rtc::VideoSinkInterface<webrtc::VideoFrame>* sink) = 0;
// Gets quality stats for the channel.
diff --git a/media/engine/webrtcvideoengine2.cc b/media/engine/webrtcvideoengine2.cc
index 087fc4e..e92598d 100644
--- a/media/engine/webrtcvideoengine2.cc
+++ b/media/engine/webrtcvideoengine2.cc
@@ -865,20 +865,33 @@
webrtc::RtpParameters WebRtcVideoChannel2::GetRtpReceiveParameters(
uint32_t ssrc) const {
+ webrtc::RtpParameters rtp_params;
rtc::CritScope stream_lock(&stream_crit_);
- auto it = receive_streams_.find(ssrc);
- if (it == receive_streams_.end()) {
- LOG(LS_WARNING) << "Attempting to get RTP receive parameters for stream "
- << "with ssrc " << ssrc << " which doesn't exist.";
- return webrtc::RtpParameters();
+ // SSRC of 0 represents an unsignaled receive stream.
+ if (ssrc == 0) {
+ if (!default_unsignalled_ssrc_handler_.GetDefaultSink()) {
+ LOG(LS_WARNING) << "Attempting to get RTP parameters for the default, "
+ "unsignaled video receive stream, but not yet "
+ "configured to receive such a stream.";
+ return rtp_params;
+ }
+ rtp_params.encodings.emplace_back();
+ } else {
+ auto it = receive_streams_.find(ssrc);
+ if (it == receive_streams_.end()) {
+ LOG(LS_WARNING) << "Attempting to get RTP receive parameters for stream "
+ << "with SSRC " << ssrc << " which doesn't exist.";
+ return webrtc::RtpParameters();
+ }
+ // TODO(deadbeef): Return stream-specific parameters, beyond just SSRC.
+ rtp_params.encodings.emplace_back();
+ rtp_params.encodings[0].ssrc = it->second->GetFirstPrimarySsrc();
}
- // TODO(deadbeef): Return stream-specific parameters.
- webrtc::RtpParameters rtp_params = CreateRtpParametersWithOneEncoding();
+ // Add codecs, which any stream is prepared to receive.
for (const VideoCodec& codec : recv_params_.codecs) {
rtp_params.codecs.push_back(codec.ToCodecParameters());
}
- rtp_params.encodings[0].ssrc = it->second->GetFirstPrimarySsrc();
return rtp_params;
}
@@ -887,11 +900,22 @@
const webrtc::RtpParameters& parameters) {
TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetRtpReceiveParameters");
rtc::CritScope stream_lock(&stream_crit_);
- auto it = receive_streams_.find(ssrc);
- if (it == receive_streams_.end()) {
- LOG(LS_ERROR) << "Attempting to set RTP receive parameters for stream "
- << "with ssrc " << ssrc << " which doesn't exist.";
- return false;
+
+ // SSRC of 0 represents an unsignaled receive stream.
+ if (ssrc == 0) {
+ if (!default_unsignalled_ssrc_handler_.GetDefaultSink()) {
+ LOG(LS_WARNING) << "Attempting to set RTP parameters for the default, "
+ "unsignaled video receive stream, but not yet "
+ "configured to receive such a stream.";
+ return false;
+ }
+ } else {
+ auto it = receive_streams_.find(ssrc);
+ if (it == receive_streams_.end()) {
+ LOG(LS_WARNING) << "Attempting to set RTP receive parameters for stream "
+ << "with SSRC " << ssrc << " which doesn't exist.";
+ return false;
+ }
}
webrtc::RtpParameters current_parameters = GetRtpReceiveParameters(ssrc);
diff --git a/media/engine/webrtcvideoengine2.h b/media/engine/webrtcvideoengine2.h
index 0a2f4a5..2c7d36c 100644
--- a/media/engine/webrtcvideoengine2.h
+++ b/media/engine/webrtcvideoengine2.h
@@ -83,6 +83,9 @@
rtc::VideoSinkInterface<webrtc::VideoFrame>* GetDefaultSink() const;
void SetDefaultSink(VideoMediaChannel* channel,
rtc::VideoSinkInterface<webrtc::VideoFrame>* sink);
+
+ uint32_t default_recv_ssrc() const { return default_recv_ssrc_; }
+
virtual ~DefaultUnsignalledSsrcHandler() = default;
private:
diff --git a/media/engine/webrtcvideoengine2_unittest.cc b/media/engine/webrtcvideoengine2_unittest.cc
index 37e227f..40d1a3f 100644
--- a/media/engine/webrtcvideoengine2_unittest.cc
+++ b/media/engine/webrtcvideoengine2_unittest.cc
@@ -4136,6 +4136,49 @@
EXPECT_EQ(initial_params, channel_->GetRtpReceiveParameters(last_ssrc_));
}
+// Test that GetRtpReceiveParameters returns parameters correctly when SSRCs
+// aren't signaled. It should always return an empty "RtpEncodingParameters",
+// even after a packet is received and the unsignaled SSRC is known.
+TEST_F(WebRtcVideoChannel2Test, GetRtpReceiveParametersWithUnsignaledSsrc) {
+ // Call necessary methods to configure receiving a default stream as
+ // soon as it arrives.
+ cricket::VideoRecvParameters parameters;
+ parameters.codecs.push_back(GetEngineCodec("VP8"));
+ parameters.codecs.push_back(GetEngineCodec("VP9"));
+ EXPECT_TRUE(channel_->SetRecvParameters(parameters));
+
+ // Call GetRtpReceiveParameters before configured to receive an unsignaled
+ // stream. Should return nothing.
+ EXPECT_EQ(webrtc::RtpParameters(), channel_->GetRtpReceiveParameters(0));
+
+ // Set a sink for an unsignaled stream.
+ cricket::FakeVideoRenderer renderer;
+ // Value of "0" means "unsignaled stream".
+ EXPECT_TRUE(channel_->SetSink(0, &renderer));
+
+ // Call GetRtpReceiveParameters before the SSRC is known. Value of "0"
+ // in this method means "unsignaled stream".
+ webrtc::RtpParameters rtp_parameters = channel_->GetRtpReceiveParameters(0);
+ ASSERT_EQ(1u, rtp_parameters.encodings.size());
+ EXPECT_FALSE(rtp_parameters.encodings[0].ssrc);
+
+ // Receive VP8 packet.
+ uint8_t data[kMinRtpPacketLen];
+ cricket::RtpHeader rtpHeader;
+ rtpHeader.payload_type = GetEngineCodec("VP8").id;
+ rtpHeader.seq_num = rtpHeader.timestamp = 0;
+ rtpHeader.ssrc = kIncomingUnsignalledSsrc;
+ cricket::SetRtpHeader(data, sizeof(data), rtpHeader);
+ rtc::CopyOnWriteBuffer packet(data, sizeof(data));
+ rtc::PacketTime packet_time;
+ channel_->OnPacketReceived(&packet, packet_time);
+
+ // The |ssrc| member should still be unset.
+ rtp_parameters = channel_->GetRtpReceiveParameters(0);
+ ASSERT_EQ(1u, rtp_parameters.encodings.size());
+ EXPECT_FALSE(rtp_parameters.encodings[0].ssrc);
+}
+
void WebRtcVideoChannel2Test::TestReceiverLocalSsrcConfiguration(
bool receiver_first) {
EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));
diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc
index 4aa97e0..2384ac2 100644
--- a/media/engine/webrtcvoiceengine.cc
+++ b/media/engine/webrtcvoiceengine.cc
@@ -1755,19 +1755,31 @@
webrtc::RtpParameters WebRtcVoiceMediaChannel::GetRtpReceiveParameters(
uint32_t ssrc) const {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
- auto it = recv_streams_.find(ssrc);
- if (it == recv_streams_.end()) {
- LOG(LS_WARNING) << "Attempting to get RTP receive parameters for stream "
- << "with ssrc " << ssrc << " which doesn't exist.";
- return webrtc::RtpParameters();
+ webrtc::RtpParameters rtp_params;
+ // SSRC of 0 represents the default receive stream.
+ if (ssrc == 0) {
+ if (!default_sink_) {
+ LOG(LS_WARNING) << "Attempting to get RTP parameters for the default, "
+ "unsignaled audio receive stream, but not yet "
+ "configured to receive such a stream.";
+ return rtp_params;
+ }
+ rtp_params.encodings.emplace_back();
+ } else {
+ auto it = recv_streams_.find(ssrc);
+ if (it == recv_streams_.end()) {
+ LOG(LS_WARNING) << "Attempting to get RTP receive parameters for stream "
+ << "with ssrc " << ssrc << " which doesn't exist.";
+ return webrtc::RtpParameters();
+ }
+ rtp_params.encodings.emplace_back();
+ // TODO(deadbeef): Return stream-specific parameters.
+ rtp_params.encodings[0].ssrc = rtc::Optional<uint32_t>(ssrc);
}
- // TODO(deadbeef): Return stream-specific parameters.
- webrtc::RtpParameters rtp_params = CreateRtpParametersWithOneEncoding();
for (const AudioCodec& codec : recv_codecs_) {
rtp_params.codecs.push_back(codec.ToCodecParameters());
}
- rtp_params.encodings[0].ssrc = rtc::Optional<uint32_t>(ssrc);
return rtp_params;
}
@@ -1775,11 +1787,21 @@
uint32_t ssrc,
const webrtc::RtpParameters& parameters) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
- auto it = recv_streams_.find(ssrc);
- if (it == recv_streams_.end()) {
- LOG(LS_WARNING) << "Attempting to set RTP receive parameters for stream "
- << "with ssrc " << ssrc << " which doesn't exist.";
- return false;
+ // SSRC of 0 represents the default receive stream.
+ if (ssrc == 0) {
+ if (!default_sink_) {
+ LOG(LS_WARNING) << "Attempting to set RTP parameters for the default, "
+ "unsignaled audio receive stream, but not yet "
+ "configured to receive such a stream.";
+ return false;
+ }
+ } else {
+ auto it = recv_streams_.find(ssrc);
+ if (it == recv_streams_.end()) {
+ LOG(LS_WARNING) << "Attempting to set RTP receive parameters for stream "
+ << "with ssrc " << ssrc << " which doesn't exist.";
+ return false;
+ }
}
webrtc::RtpParameters current_parameters = GetRtpReceiveParameters(ssrc);
@@ -2320,6 +2342,7 @@
bool WebRtcVoiceMediaChannel::SetOutputVolume(uint32_t ssrc, double volume) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
std::vector<uint32_t> ssrcs(1, ssrc);
+ // SSRC of 0 represents the default receive stream.
if (ssrc == 0) {
default_recv_volume_ = volume;
ssrcs = unsignaled_recv_ssrcs_;
diff --git a/media/engine/webrtcvoiceengine_unittest.cc b/media/engine/webrtcvoiceengine_unittest.cc
index f9a631c..75b6320 100644
--- a/media/engine/webrtcvoiceengine_unittest.cc
+++ b/media/engine/webrtcvoiceengine_unittest.cc
@@ -1238,6 +1238,43 @@
EXPECT_EQ(initial_params, channel_->GetRtpReceiveParameters(kSsrcX));
}
+// Test that GetRtpReceiveParameters returns parameters correctly when SSRCs
+// aren't signaled. It should return an empty "RtpEncodingParameters" when
+// configured to receive an unsignaled stream and no packets have been received
+// yet, and start returning the SSRC once a packet has been received.
+TEST_F(WebRtcVoiceEngineTestFake, GetRtpReceiveParametersWithUnsignaledSsrc) {
+ ASSERT_TRUE(SetupChannel());
+ // Call necessary methods to configure receiving a default stream as
+ // soon as it arrives.
+ cricket::AudioRecvParameters parameters;
+ parameters.codecs.push_back(kIsacCodec);
+ parameters.codecs.push_back(kPcmuCodec);
+ EXPECT_TRUE(channel_->SetRecvParameters(parameters));
+
+ // Call GetRtpReceiveParameters before configured to receive an unsignaled
+ // stream. Should return nothing.
+ EXPECT_EQ(webrtc::RtpParameters(), channel_->GetRtpReceiveParameters(0));
+
+ // Set a sink for an unsignaled stream.
+ std::unique_ptr<FakeAudioSink> fake_sink(new FakeAudioSink());
+ // Value of "0" means "unsignaled stream".
+ channel_->SetRawAudioSink(0, std::move(fake_sink));
+
+ // Call GetRtpReceiveParameters before the SSRC is known. Value of "0"
+ // in this method means "unsignaled stream".
+ webrtc::RtpParameters rtp_parameters = channel_->GetRtpReceiveParameters(0);
+ ASSERT_EQ(1u, rtp_parameters.encodings.size());
+ EXPECT_FALSE(rtp_parameters.encodings[0].ssrc);
+
+ // Receive PCMU packet (SSRC=1).
+ DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
+
+ // The |ssrc| member should still be unset.
+ rtp_parameters = channel_->GetRtpReceiveParameters(0);
+ ASSERT_EQ(1u, rtp_parameters.encodings.size());
+ EXPECT_FALSE(rtp_parameters.encodings[0].ssrc);
+}
+
// Test that we apply codecs properly.
TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecs) {
EXPECT_TRUE(SetupSendStream());
