From d1ea06b3d5adab352741df5092c56b20f3e1a74f Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Fri, 18 Jul 2014 09:35:58 +0000 Subject: [PATCH] Restart VideoReceiveStreams in WebRtcVideoEngine2. Puts VideoReceiveStreams in a wrapper, WebRtcVideoReceiveStream that contain their state (configs). WebRtcVideoRenderer (the wrapper between webrtc::VideoRenderer and cricket::VideoRenderer) has also been merged into WebRtcVideoReceiveStream. Implements and tests setting codecs with new FEC settings as well as RTP header extensions on already existing receive streams. BUG=1788 R=pthatcher@webrtc.org, wu@webrtc.org Review URL: https://webrtc-codereview.appspot.com/14879004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6727 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/media/webrtc/webrtcvideoengine2.cc | 339 ++++++++++-------- talk/media/webrtc/webrtcvideoengine2.h | 73 ++-- .../webrtc/webrtcvideoengine2_unittest.cc | 55 ++- 3 files changed, 284 insertions(+), 183 deletions(-) diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index 32c93cf69..03c5e145f 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -638,50 +638,6 @@ class WebRtcVideoRenderFrame : public VideoFrame { const webrtc::I420VideoFrame* const frame_; }; -WebRtcVideoRenderer::WebRtcVideoRenderer() - : last_width_(-1), last_height_(-1), renderer_(NULL) {} - -void WebRtcVideoRenderer::RenderFrame(const webrtc::I420VideoFrame& frame, - int time_to_render_ms) { - talk_base::CritScope crit(&lock_); - if (renderer_ == NULL) { - LOG(LS_WARNING) << "VideoReceiveStream not connected to a VideoRenderer."; - return; - } - - if (frame.width() != last_width_ || frame.height() != last_height_) { - SetSize(frame.width(), frame.height()); - } - - LOG(LS_VERBOSE) << "RenderFrame: (" << frame.width() << "x" << frame.height() - << ")"; - - const WebRtcVideoRenderFrame render_frame(&frame); - renderer_->RenderFrame(&render_frame); -} - -void WebRtcVideoRenderer::SetRenderer(cricket::VideoRenderer* renderer) { - talk_base::CritScope crit(&lock_); - renderer_ = renderer; - if (renderer_ != NULL && last_width_ != -1) { - SetSize(last_width_, last_height_); - } -} - -VideoRenderer* WebRtcVideoRenderer::GetRenderer() { - talk_base::CritScope crit(&lock_); - return renderer_; -} - -void WebRtcVideoRenderer::SetSize(int width, int height) { - talk_base::CritScope crit(&lock_); - if (!renderer_->SetSize(width, height, 0)) { - LOG(LS_ERROR) << "Could not set renderer size."; - } - last_width_ = width; - last_height_ = height; -} - // WebRtcVideoChannel2 WebRtcVideoChannel2::WebRtcVideoChannel2( @@ -720,18 +676,10 @@ WebRtcVideoChannel2::~WebRtcVideoChannel2() { delete it->second; } - for (std::map::iterator it = + for (std::map::iterator it = receive_streams_.begin(); it != receive_streams_.end(); ++it) { - assert(it->second != NULL); - call_->DestroyVideoReceiveStream(it->second); - } - - for (std::map::iterator it = renderers_.begin(); - it != renderers_.end(); - ++it) { - assert(it->second != NULL); delete it->second; } } @@ -812,6 +760,14 @@ bool WebRtcVideoChannel2::SetRecvCodecs(const std::vector& codecs) { } recv_codecs_ = mapped_codecs; + + for (std::map::iterator it = + receive_streams_.begin(); + it != receive_streams_.end(); + ++it) { + it->second->SetRecvCodecs(recv_codecs_); + } + return true; } @@ -832,7 +788,13 @@ bool WebRtcVideoChannel2::SetSendCodecs(const std::vector& codecs) { send_codec_.Set(supported_codecs.front()); LOG(LS_INFO) << "Using codec: " << supported_codecs.front().codec.ToString(); - SetCodecForAllSendStreams(supported_codecs.front()); + for (std::map::iterator it = + send_streams_.begin(); + it != send_streams_.end(); + ++it) { + assert(it->second != NULL); + it->second->SetCodec(supported_codecs.front()); + } return true; } @@ -974,87 +936,52 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp) { } webrtc::VideoReceiveStream::Config config; - config.rtp.remote_ssrc = ssrc; - config.rtp.local_ssrc = rtcp_receiver_report_ssrc_; + ConfigureReceiverRtp(&config, sp); + receive_streams_[ssrc] = + new WebRtcVideoReceiveStream(call_.get(), config, recv_codecs_); + + return true; +} + +void WebRtcVideoChannel2::ConfigureReceiverRtp( + webrtc::VideoReceiveStream::Config* config, + const StreamParams& sp) const { + uint32 ssrc = sp.first_ssrc(); + + config->rtp.remote_ssrc = ssrc; + config->rtp.local_ssrc = rtcp_receiver_report_ssrc_; if (IsNackEnabled(recv_codecs_.begin()->codec)) { - config.rtp.nack.rtp_history_ms = kNackHistoryMs; + config->rtp.nack.rtp_history_ms = kNackHistoryMs; } - config.rtp.remb = true; - config.rtp.extensions = recv_rtp_extensions_; + config->rtp.remb = true; + config->rtp.extensions = recv_rtp_extensions_; // TODO(pbos): This protection is against setting the same local ssrc as // remote which is not permitted by the lower-level API. RTCP requires a // corresponding sender SSRC. Figure out what to do when we don't have // (receive-only) or know a good local SSRC. - if (config.rtp.remote_ssrc == config.rtp.local_ssrc) { - if (config.rtp.local_ssrc != kDefaultRtcpReceiverReportSsrc) { - config.rtp.local_ssrc = kDefaultRtcpReceiverReportSsrc; + if (config->rtp.remote_ssrc == config->rtp.local_ssrc) { + if (config->rtp.local_ssrc != kDefaultRtcpReceiverReportSsrc) { + config->rtp.local_ssrc = kDefaultRtcpReceiverReportSsrc; } else { - config.rtp.local_ssrc = kDefaultRtcpReceiverReportSsrc + 1; + config->rtp.local_ssrc = kDefaultRtcpReceiverReportSsrc + 1; } } - bool default_renderer_used = false; - for (std::map::iterator it = renderers_.begin(); - it != renderers_.end(); - ++it) { - if (it->second->GetRenderer() == default_renderer_) { - default_renderer_used = true; + + for (size_t i = 0; i < recv_codecs_.size(); ++i) { + if (recv_codecs_[i].codec.id == kDefaultVideoCodecPref.payload_type) { + config->rtp.fec = recv_codecs_[i].fec; + uint32 rtx_ssrc; + if (recv_codecs_[i].rtx_payload_type != -1 && + sp.GetFidSsrc(ssrc, &rtx_ssrc)) { + config->rtp.rtx[kDefaultVideoCodecPref.payload_type].ssrc = rtx_ssrc; + config->rtp.rtx[kDefaultVideoCodecPref.payload_type].payload_type = + recv_codecs_[i].rtx_payload_type; + } break; } } - assert(renderers_[ssrc] == NULL); - renderers_[ssrc] = new WebRtcVideoRenderer(); - if (!default_renderer_used) { - renderers_[ssrc]->SetRenderer(default_renderer_); - } - config.renderer = renderers_[ssrc]; - - { - // TODO(pbos): Base receive codecs off recv_codecs_ and set up using a - // DecoderFactory similar to send side. Pending webrtc:2854. - // Also set up default codecs if there's nothing in recv_codecs_. - webrtc::VideoCodec codec; - memset(&codec, 0, sizeof(codec)); - - codec.plType = kDefaultVideoCodecPref.payload_type; - talk_base::strcpyn(codec.plName, ARRAY_SIZE(codec.plName), - kDefaultVideoCodecPref.name); - codec.codecType = webrtc::kVideoCodecVP8; - codec.codecSpecific.VP8.resilience = webrtc::kResilientStream; - codec.codecSpecific.VP8.numberOfTemporalLayers = 1; - codec.codecSpecific.VP8.denoisingOn = true; - codec.codecSpecific.VP8.errorConcealmentOn = false; - codec.codecSpecific.VP8.automaticResizeOn = false; - codec.codecSpecific.VP8.frameDroppingOn = true; - codec.codecSpecific.VP8.keyFrameInterval = 3000; - // Bitrates don't matter and are ignored for the receiver. This is put in to - // have the current underlying implementation accept the VideoCodec. - codec.minBitrate = codec.startBitrate = codec.maxBitrate = 300; - config.codecs.push_back(codec); - for (size_t i = 0; i < recv_codecs_.size(); ++i) { - if (recv_codecs_[i].codec.id == codec.plType) { - config.rtp.fec = recv_codecs_[i].fec; - uint32 rtx_ssrc; - if (recv_codecs_[i].rtx_payload_type != -1 && - sp.GetFidSsrc(ssrc, &rtx_ssrc)) { - config.rtp.rtx[codec.plType].ssrc = rtx_ssrc; - config.rtp.rtx[codec.plType].payload_type = - recv_codecs_[i].rtx_payload_type; - } - break; - } - } - } - - webrtc::VideoReceiveStream* receive_stream = - call_->CreateVideoReceiveStream(config); - assert(receive_stream != NULL); - - receive_streams_[ssrc] = receive_stream; - receive_stream->Start(); - - return true; } bool WebRtcVideoChannel2::RemoveRecvStream(uint32 ssrc) { @@ -1063,21 +990,15 @@ bool WebRtcVideoChannel2::RemoveRecvStream(uint32 ssrc) { ssrc = default_recv_ssrc_; } - std::map::iterator stream = + std::map::iterator stream = receive_streams_.find(ssrc); if (stream == receive_streams_.end()) { LOG(LS_ERROR) << "Stream not found for ssrc: " << ssrc; return false; } - call_->DestroyVideoReceiveStream(stream->second); + delete stream->second; receive_streams_.erase(stream); - std::map::iterator renderer = - renderers_.find(ssrc); - assert(renderer != renderers_.end()); - delete renderer->second; - renderers_.erase(renderer); - if (ssrc == default_recv_ssrc_) { default_recv_ssrc_ = 0; } @@ -1088,16 +1009,19 @@ bool WebRtcVideoChannel2::RemoveRecvStream(uint32 ssrc) { bool WebRtcVideoChannel2::SetRenderer(uint32 ssrc, VideoRenderer* renderer) { LOG(LS_INFO) << "SetRenderer: ssrc:" << ssrc << " " << (renderer ? "(ptr)" : "NULL"); - bool is_default_ssrc = false; if (ssrc == 0) { - is_default_ssrc = true; + if (default_recv_ssrc_!= 0) { + receive_streams_[default_recv_ssrc_]->SetRenderer(renderer); + } ssrc = default_recv_ssrc_; default_renderer_ = renderer; + return true; } - std::map::iterator it = renderers_.find(ssrc); - if (it == renderers_.end()) { - return is_default_ssrc; + std::map::iterator it = + receive_streams_.find(ssrc); + if (it == receive_streams_.end()) { + return false; } it->second->SetRenderer(renderer); @@ -1113,8 +1037,9 @@ bool WebRtcVideoChannel2::GetRenderer(uint32 ssrc, VideoRenderer** renderer) { return true; } - std::map::iterator it = renderers_.find(ssrc); - if (it == renderers_.end()) { + std::map::iterator it = + receive_streams_.find(ssrc); + if (it == receive_streams_.end()) { return false; } *renderer = it->second->GetRenderer(); @@ -1179,6 +1104,7 @@ void WebRtcVideoChannel2::OnPacketReceived( sp.ssrcs.push_back(ssrc); LOG(LS_INFO) << "Creating default receive stream for SSRC=" << ssrc << "."; AddRecvStream(sp); + SetRenderer(0, default_renderer_); if (call_->Receiver()->DeliverPacket( reinterpret_cast(packet->data()), packet->length()) != @@ -1226,6 +1152,14 @@ bool WebRtcVideoChannel2::SetRecvRtpHeaderExtensions( webrtc_extensions.push_back(webrtc_extension); } recv_rtp_extensions_ = webrtc_extensions; + + for (std::map::iterator it = + receive_streams_.begin(); + it != receive_streams_.end(); + ++it) { + it->second->SetRtpExtensions(recv_rtp_extensions_); + } + return true; } @@ -1241,6 +1175,12 @@ bool WebRtcVideoChannel2::SetSendRtpHeaderExtensions( webrtc_extensions.push_back(webrtc_extension); } send_rtp_extensions_ = webrtc_extensions; + for (std::map::iterator it = + send_streams_.begin(); + it != send_streams_.end(); + ++it) { + it->second->SetRtpExtensions(send_rtp_extensions_); + } return true; } @@ -1319,17 +1259,6 @@ void WebRtcVideoChannel2::StopAllSendStreams() { } } -void WebRtcVideoChannel2::SetCodecForAllSendStreams( - const WebRtcVideoChannel2::VideoCodecSettings& codec) { - for (std::map::iterator it = - send_streams_.begin(); - it != send_streams_.end(); - ++it) { - assert(it->second != NULL); - it->second->SetCodec(codec); - } -} - WebRtcVideoChannel2::WebRtcVideoSendStream::VideoSendStreamParameters:: VideoSendStreamParameters( const webrtc::VideoSendStream::Config& config, @@ -1577,6 +1506,13 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodecAndOptions( delete old_encoder; } +void WebRtcVideoChannel2::WebRtcVideoSendStream::SetRtpExtensions( + const std::vector& rtp_extensions) { + talk_base::CritScope cs(&lock_); + parameters_.config.rtp.extensions = rtp_extensions; + RecreateWebRtcStream(); +} + void WebRtcVideoChannel2::WebRtcVideoSendStream::SetDimensions(int width, int height) { assert(!parameters_.video_streams.empty()); @@ -1626,6 +1562,115 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::RecreateWebRtcStream() { } } +WebRtcVideoChannel2::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream( + webrtc::Call* call, + const webrtc::VideoReceiveStream::Config& config, + const std::vector& recv_codecs) + : call_(call), + config_(config), + stream_(NULL), + last_width_(-1), + last_height_(-1), + renderer_(NULL) { + config_.renderer = this; + // SetRecvCodecs will also reset (start) the VideoReceiveStream. + SetRecvCodecs(recv_codecs); +} + +WebRtcVideoChannel2::WebRtcVideoReceiveStream::~WebRtcVideoReceiveStream() { + call_->DestroyVideoReceiveStream(stream_); +} + +void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvCodecs( + const std::vector& recv_codecs) { + // TODO(pbos): Reconfigure RTX based on incoming recv_codecs. + // TODO(pbos): Base receive codecs off recv_codecs_ and set up using a + // DecoderFactory similar to send side. Pending webrtc:2854. + // Also set up default codecs if there's nothing in recv_codecs_. + webrtc::VideoCodec codec; + memset(&codec, 0, sizeof(codec)); + + codec.plType = kDefaultVideoCodecPref.payload_type; + strcpy(codec.plName, kDefaultVideoCodecPref.name); + codec.codecType = webrtc::kVideoCodecVP8; + codec.codecSpecific.VP8.resilience = webrtc::kResilientStream; + codec.codecSpecific.VP8.numberOfTemporalLayers = 1; + codec.codecSpecific.VP8.denoisingOn = true; + codec.codecSpecific.VP8.errorConcealmentOn = false; + codec.codecSpecific.VP8.automaticResizeOn = false; + codec.codecSpecific.VP8.frameDroppingOn = true; + codec.codecSpecific.VP8.keyFrameInterval = 3000; + // Bitrates don't matter and are ignored for the receiver. This is put in to + // have the current underlying implementation accept the VideoCodec. + codec.minBitrate = codec.startBitrate = codec.maxBitrate = 300; + config_.codecs.clear(); + config_.codecs.push_back(codec); + + config_.rtp.fec = recv_codecs.front().fec; + + RecreateWebRtcStream(); +} + +void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRtpExtensions( + const std::vector& extensions) { + config_.rtp.extensions = extensions; + RecreateWebRtcStream(); +} + +void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() { + if (stream_ != NULL) { + call_->DestroyVideoReceiveStream(stream_); + } + stream_ = call_->CreateVideoReceiveStream(config_); + stream_->Start(); +} + +void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RenderFrame( + const webrtc::I420VideoFrame& frame, + int time_to_render_ms) { + talk_base::CritScope crit(&renderer_lock_); + if (renderer_ == NULL) { + LOG(LS_WARNING) << "VideoReceiveStream not connected to a VideoRenderer."; + return; + } + + if (frame.width() != last_width_ || frame.height() != last_height_) { + SetSize(frame.width(), frame.height()); + } + + LOG(LS_VERBOSE) << "RenderFrame: (" << frame.width() << "x" << frame.height() + << ")"; + + const WebRtcVideoRenderFrame render_frame(&frame); + renderer_->RenderFrame(&render_frame); +} + +void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRenderer( + cricket::VideoRenderer* renderer) { + talk_base::CritScope crit(&renderer_lock_); + renderer_ = renderer; + if (renderer_ != NULL && last_width_ != -1) { + SetSize(last_width_, last_height_); + } +} + +VideoRenderer* WebRtcVideoChannel2::WebRtcVideoReceiveStream::GetRenderer() { + // TODO(pbos): Remove GetRenderer and all uses of it, it's thread-unsafe by + // design. + talk_base::CritScope crit(&renderer_lock_); + return renderer_; +} + +void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetSize(int width, + int height) { + talk_base::CritScope crit(&renderer_lock_); + if (!renderer_->SetSize(width, height, 0)) { + LOG(LS_ERROR) << "Could not set renderer size."; + } + last_width_ = width; + last_height_ = height; +} + WebRtcVideoChannel2::VideoCodecSettings::VideoCodecSettings() : rtx_payload_type(-1) {} diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h index 51223907c..fe798b50c 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -41,6 +41,7 @@ #include "webrtc/transport.h" #include "webrtc/video_renderer.h" #include "webrtc/video_send_stream.h" +#include "webrtc/video_receive_stream.h" namespace webrtc { class Call; @@ -170,29 +171,6 @@ class WebRtcVideoEngine2 : public sigslot::has_slots<> { WebRtcVideoEncoderFactory2 default_video_encoder_factory_; }; -// Adapter between webrtc::VideoRenderer and cricket::VideoRenderer. -// The webrtc::VideoRenderer is set once, whereas the cricket::VideoRenderer can -// be set after initialization. This adapter will also convert the incoming -// webrtc::I420VideoFrame to a frame type that cricket::VideoRenderer can -// render. -class WebRtcVideoRenderer : public webrtc::VideoRenderer { - public: - WebRtcVideoRenderer(); - - virtual void RenderFrame(const webrtc::I420VideoFrame& frame, - int time_to_render_ms) OVERRIDE; - - void SetRenderer(cricket::VideoRenderer* renderer); - cricket::VideoRenderer* GetRenderer(); - - private: - void SetSize(int width, int height); - int last_width_; - int last_height_; - talk_base::CriticalSection lock_; - cricket::VideoRenderer* renderer_ GUARDED_BY(lock_); -}; - class WebRtcVideoChannel2 : public talk_base::MessageHandler, public VideoMediaChannel, public webrtc::newapi::Transport { @@ -261,6 +239,9 @@ class WebRtcVideoChannel2 : public talk_base::MessageHandler, bool GetRenderer(uint32 ssrc, VideoRenderer** renderer); private: + void ConfigureReceiverRtp(webrtc::VideoReceiveStream::Config* config, + const StreamParams& sp) const; + struct VideoCodecSettings { VideoCodecSettings(); @@ -269,6 +250,8 @@ class WebRtcVideoChannel2 : public talk_base::MessageHandler, int rtx_payload_type; }; + // Wrapper for the sender part, this is where the capturer is connected and + // frames are then converted from cricket frames to webrtc frames. class WebRtcVideoSendStream : public sigslot::has_slots<> { public: WebRtcVideoSendStream( @@ -282,6 +265,8 @@ class WebRtcVideoChannel2 : public talk_base::MessageHandler, ~WebRtcVideoSendStream(); void SetOptions(const VideoOptions& options); void SetCodec(const VideoCodecSettings& codec); + void SetRtpExtensions( + const std::vector& rtp_extensions); void InputFrame(VideoCapturer* capturer, const VideoFrame* frame); bool SetCapturer(VideoCapturer* capturer); @@ -332,6 +317,41 @@ class WebRtcVideoChannel2 : public talk_base::MessageHandler, webrtc::I420VideoFrame video_frame_ GUARDED_BY(frame_lock_); }; + // Wrapper for the receiver part, contains configs etc. that are needed to + // reconstruct the underlying VideoReceiveStream. Also serves as a wrapper + // between webrtc::VideoRenderer and cricket::VideoRenderer. + class WebRtcVideoReceiveStream : public webrtc::VideoRenderer { + public: + WebRtcVideoReceiveStream( + webrtc::Call*, + const webrtc::VideoReceiveStream::Config& config, + const std::vector& recv_codecs); + ~WebRtcVideoReceiveStream(); + + void SetRecvCodecs(const std::vector& recv_codecs); + void SetRtpExtensions(const std::vector& extensions); + + virtual void RenderFrame(const webrtc::I420VideoFrame& frame, + int time_to_render_ms) OVERRIDE; + + void SetRenderer(cricket::VideoRenderer* renderer); + cricket::VideoRenderer* GetRenderer(); + + private: + void SetSize(int width, int height); + void RecreateWebRtcStream(); + + webrtc::Call* const call_; + + webrtc::VideoReceiveStream* stream_; + webrtc::VideoReceiveStream::Config config_; + + talk_base::CriticalSection renderer_lock_; + cricket::VideoRenderer* renderer_ GUARDED_BY(renderer_lock_); + int last_width_; + int last_height_; + }; + void Construct(webrtc::Call* call, WebRtcVideoEngine2* engine); virtual bool SendRtp(const uint8_t* data, size_t len) OVERRIDE; @@ -339,7 +359,7 @@ class WebRtcVideoChannel2 : public talk_base::MessageHandler, void StartAllSendStreams(); void StopAllSendStreams(); - void SetCodecForAllSendStreams(const VideoCodecSettings& codec); + static std::vector MapCodecs( const std::vector& codecs); std::vector FilterSupportedCodecs( @@ -348,14 +368,13 @@ class WebRtcVideoChannel2 : public talk_base::MessageHandler, uint32_t rtcp_receiver_report_ssrc_; bool sending_; talk_base::scoped_ptr call_; - std::map renderers_; - VideoRenderer* default_renderer_; uint32_t default_send_ssrc_; uint32_t default_recv_ssrc_; + VideoRenderer* default_renderer_; // Using primary-ssrc (first ssrc) as key. std::map send_streams_; - std::map receive_streams_; + std::map receive_streams_; Settable send_codec_; std::vector send_rtp_extensions_; diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index 28abbd1de..d272929e4 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -621,6 +621,7 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test { void TestSetSendRtpHeaderExtensions(const std::string& cricket_ext, const std::string& webrtc_ext) { + FakeCall* call = fake_channel_->GetFakeCall(); // Enable extension. const int id = 1; std::vector extensions; @@ -642,15 +643,25 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test { ->GetConfig() .rtp.extensions.empty()); - // Remove the extension id, verify that this doesn't reset extensions as - // they should be set before creating channels. + // Verify that existing RTP header extensions can be removed. std::vector empty_extensions; EXPECT_TRUE(channel_->SetSendRtpHeaderExtensions(empty_extensions)); - EXPECT_FALSE(send_stream->GetConfig().rtp.extensions.empty()); + ASSERT_EQ(1u, call->GetVideoSendStreams().size()); + send_stream = call->GetVideoSendStreams()[0]; + EXPECT_TRUE(send_stream->GetConfig().rtp.extensions.empty()); + + // Verify that adding receive RTP header extensions adds them for existing + // streams. + EXPECT_TRUE(channel_->SetSendRtpHeaderExtensions(extensions)); + send_stream = call->GetVideoSendStreams()[0]; + ASSERT_EQ(1u, send_stream->GetConfig().rtp.extensions.size()); + EXPECT_EQ(id, send_stream->GetConfig().rtp.extensions[0].id); + EXPECT_EQ(webrtc_ext, send_stream->GetConfig().rtp.extensions[0].name); } void TestSetRecvRtpHeaderExtensions(const std::string& cricket_ext, const std::string& webrtc_ext) { + FakeCall* call = fake_channel_->GetFakeCall(); // Enable extension. const int id = 1; std::vector extensions; @@ -666,17 +677,27 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test { EXPECT_EQ(webrtc_ext, recv_stream->GetConfig().rtp.extensions[0].name); // Verify call with same set of extensions returns true. EXPECT_TRUE(channel_->SetRecvRtpHeaderExtensions(extensions)); + // Verify that SetRecvRtpHeaderExtensions doesn't implicitly add them for // senders. EXPECT_TRUE(AddSendStream(cricket::StreamParams::CreateLegacy(123)) ->GetConfig() .rtp.extensions.empty()); - // Remove the extension id, verify that this doesn't reset extensions as - // they should be set before creating channels. + // Verify that existing RTP header extensions can be removed. std::vector empty_extensions; - EXPECT_TRUE(channel_->SetSendRtpHeaderExtensions(empty_extensions)); - EXPECT_FALSE(recv_stream->GetConfig().rtp.extensions.empty()); + EXPECT_TRUE(channel_->SetRecvRtpHeaderExtensions(empty_extensions)); + ASSERT_EQ(1u, call->GetVideoReceiveStreams().size()); + recv_stream = call->GetVideoReceiveStreams()[0]; + EXPECT_TRUE(recv_stream->GetConfig().rtp.extensions.empty()); + + // Verify that adding receive RTP header extensions adds them for existing + // streams. + EXPECT_TRUE(channel_->SetRecvRtpHeaderExtensions(extensions)); + recv_stream = call->GetVideoReceiveStreams()[0]; + ASSERT_EQ(1u, recv_stream->GetConfig().rtp.extensions.size()); + EXPECT_EQ(id, recv_stream->GetConfig().rtp.extensions[0].id); + EXPECT_EQ(webrtc_ext, recv_stream->GetConfig().rtp.extensions[0].name); } talk_base::scoped_ptr channel_; @@ -1230,8 +1251,24 @@ TEST_F(WebRtcVideoChannel2Test, FAIL(); // TODO(pbos): Verify that the FEC parameters are set for all codecs. } -TEST_F(WebRtcVideoChannel2Test, DISABLED_SetRecvCodecsWithoutFecDisablesFec) { - FAIL() << "Not implemented."; // TODO(pbos): Implement. +TEST_F(WebRtcVideoChannel2Test, SetRecvCodecsWithoutFecDisablesFec) { + std::vector codecs; + codecs.push_back(kVp8Codec); + codecs.push_back(kUlpfecCodec); + ASSERT_TRUE(channel_->SetSendCodecs(codecs)); + + FakeVideoReceiveStream* stream = AddRecvStream(); + webrtc::VideoReceiveStream::Config config = stream->GetConfig(); + + EXPECT_EQ(kUlpfecCodec.id, config.rtp.fec.ulpfec_payload_type); + + codecs.pop_back(); + ASSERT_TRUE(channel_->SetRecvCodecs(codecs)); + stream = fake_channel_->GetFakeCall()->GetVideoReceiveStreams()[0]; + ASSERT_TRUE(stream != NULL); + config = stream->GetConfig(); + EXPECT_EQ(-1, config.rtp.fec.ulpfec_payload_type) + << "SetSendCodec without FEC should disable current FEC."; } TEST_F(WebRtcVideoChannel2Test, SetSendCodecsRejectDuplicateFecPayloads) {