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-Commit-Position: refs/heads/master@{#17803}
This commit is contained in:
deadbeef 2017-04-20 19:25:07 -07:00 committed by Commit bot
parent f96439a5da
commit 3bc15103ae
6 changed files with 167 additions and 27 deletions

View File

@ -973,6 +973,11 @@ class VoiceMediaChannel : public MediaChannel {
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 @@ class VideoMediaChannel : public MediaChannel {
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 @@ class VideoMediaChannel : public MediaChannel {
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.

View File

@ -865,20 +865,33 @@ bool WebRtcVideoChannel2::SetRtpSendParameters(
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 @@ bool WebRtcVideoChannel2::SetRtpReceiveParameters(
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);

View File

@ -83,6 +83,9 @@ class DefaultUnsignalledSsrcHandler : public UnsignalledSsrcHandler {
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:

View File

@ -4136,6 +4136,49 @@ TEST_F(WebRtcVideoChannel2Test, SetAndGetRtpReceiveParameters) {
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_));

View File

@ -1755,19 +1755,31 @@ bool WebRtcVoiceMediaChannel::SetRtpSendParameters(
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 @@ bool WebRtcVoiceMediaChannel::SetRtpReceiveParameters(
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 @@ int WebRtcVoiceMediaChannel::GetOutputLevel() {
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_;

View File

@ -1238,6 +1238,43 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAndGetRtpReceiveParameters) {
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());