From 15e4e34872472fbd739a1bd487903779a068688b Mon Sep 17 00:00:00 2001 From: "mflodman@webrtc.org" Date: Mon, 8 Oct 2012 18:58:14 +0000 Subject: [PATCH] Wire up ssrc check in ViEEncoder for intra requests. Review URL: https://webrtc-codereview.appspot.com/872004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@2884 4adac7df-926f-26a2-2b94-8c16560cd09d --- src/video_engine/encoder_state_feedback.cc | 19 +++-- src/video_engine/encoder_state_feedback.h | 2 +- .../encoder_state_feedback_unittest.cc | 8 +- src/video_engine/vie_channel.cc | 20 ++++- src/video_engine/vie_channel.h | 4 +- src/video_engine/vie_channel_manager.cc | 33 ++++++-- src/video_engine/vie_channel_manager.h | 4 + src/video_engine/vie_codec_impl.cc | 28 +++++++ src/video_engine/vie_encoder.cc | 75 ++++++++++++++++--- src/video_engine/vie_encoder.h | 9 ++- src/video_engine/vie_rtp_rtcp_impl.cc | 3 +- 11 files changed, 171 insertions(+), 34 deletions(-) diff --git a/src/video_engine/encoder_state_feedback.cc b/src/video_engine/encoder_state_feedback.cc index 64e32e2d3..cfd0984bb 100644 --- a/src/video_engine/encoder_state_feedback.cc +++ b/src/video_engine/encoder_state_feedback.cc @@ -64,13 +64,16 @@ bool EncoderStateFeedback::AddEncoder(uint32_t ssrc, ViEEncoder* encoder) { return true; } -void EncoderStateFeedback::RemoveEncoder(uint32_t ssrc) { +void EncoderStateFeedback::RemoveEncoder(const ViEEncoder* encoder) { CriticalSectionScoped lock(crit_.get()); - SsrcEncoderMap::iterator it = encoders_.find(ssrc); - if (it == encoders_.end()) - return; - - encoders_.erase(it); + SsrcEncoderMap::iterator it = encoders_.begin(); + while (it != encoders_.end()) { + if (it->second == encoder) { + encoders_.erase(it++); + } else { + ++it; + } + } } RtcpIntraFrameObserver* EncoderStateFeedback::GetRtcpIntraFrameObserver() { @@ -112,8 +115,10 @@ void EncoderStateFeedback::OnLocalSsrcChanged(uint32_t old_ssrc, return; } - encoders_[new_ssrc] = it->second; + ViEEncoder* encoder = it->second; encoders_.erase(it); + encoders_[new_ssrc] = encoder; + encoder->OnLocalSsrcChanged(old_ssrc, new_ssrc); } } // namespace webrtc diff --git a/src/video_engine/encoder_state_feedback.h b/src/video_engine/encoder_state_feedback.h index 45db92fba..04d8205f3 100644 --- a/src/video_engine/encoder_state_feedback.h +++ b/src/video_engine/encoder_state_feedback.h @@ -38,7 +38,7 @@ class EncoderStateFeedback { bool AddEncoder(uint32_t ssrc, ViEEncoder* encoder); // Removes a registered ViEEncoder. - void RemoveEncoder(uint32_t ssrc); + void RemoveEncoder(const ViEEncoder* encoder); // Returns an observer to register at the requesting class. The observer has // the same lifetime as the EncoderStateFeedback instance. diff --git a/src/video_engine/encoder_state_feedback_unittest.cc b/src/video_engine/encoder_state_feedback_unittest.cc index 07d1663ea..4133a7128 100644 --- a/src/video_engine/encoder_state_feedback_unittest.cc +++ b/src/video_engine/encoder_state_feedback_unittest.cc @@ -81,7 +81,7 @@ TEST_F(VieKeyRequestTest, CreateAndTriggerRequests) { encoder_state_feedback_->GetRtcpIntraFrameObserver()->OnReceivedRPSI( ssrc, rpsi_picture_id); - encoder_state_feedback_->RemoveEncoder(ssrc); + encoder_state_feedback_->RemoveEncoder(&encoder); } // Register multiple encoders and make sure the request is relayed to correct @@ -125,12 +125,12 @@ TEST_F(VieKeyRequestTest, MultipleEncoders) { encoder_state_feedback_->GetRtcpIntraFrameObserver()->OnReceivedRPSI( ssrc_2, rpsi_pid_2); - encoder_state_feedback_->RemoveEncoder(ssrc_1); + encoder_state_feedback_->RemoveEncoder(&encoder_1); EXPECT_CALL(encoder_2, OnReceivedIntraFrameRequest(ssrc_2)) .Times(1); encoder_state_feedback_->GetRtcpIntraFrameObserver()-> OnReceivedIntraFrameRequest(ssrc_2); - encoder_state_feedback_->RemoveEncoder(ssrc_2); + encoder_state_feedback_->RemoveEncoder(&encoder_2); } TEST_F(VieKeyRequestTest, AddTwiceError) { @@ -138,7 +138,7 @@ TEST_F(VieKeyRequestTest, AddTwiceError) { MockVieEncoder encoder(process_thread_.get()); EXPECT_TRUE(encoder_state_feedback_->AddEncoder(ssrc, &encoder)); EXPECT_FALSE(encoder_state_feedback_->AddEncoder(ssrc, &encoder)); - encoder_state_feedback_->RemoveEncoder(ssrc); + encoder_state_feedback_->RemoveEncoder(&encoder); } } // namespace webrtc diff --git a/src/video_engine/vie_channel.cc b/src/video_engine/vie_channel.cc index 21cf32d41..faa3896d5 100644 --- a/src/video_engine/vie_channel.cc +++ b/src/video_engine/vie_channel.cc @@ -804,10 +804,26 @@ WebRtc_Word32 ViEChannel::SetRemoteSSRCType(const StreamType usage, return rtp_rtcp_->SetRTXReceiveStatus(true, SSRC); } -WebRtc_Word32 ViEChannel::GetLocalSSRC(uint32_t* ssrc) { +// TODO(mflodman) Add kViEStreamTypeRtx. +WebRtc_Word32 ViEChannel::GetLocalSSRC(uint8_t idx, unsigned int* ssrc) { WEBRTC_TRACE(kTraceInfo, kTraceVideo, ViEId(engine_id_, channel_id_), "%s", __FUNCTION__); - *ssrc = rtp_rtcp_->SSRC(); + + if (idx == 0) { + *ssrc = rtp_rtcp_->SSRC(); + return 0; + } + CriticalSectionScoped cs(rtp_rtcp_cs_.get()); + if (idx > simulcast_rtp_rtcp_.size()) { + return -1; + } + std::list::const_iterator it = simulcast_rtp_rtcp_.begin(); + for (int i = 1; i < idx; ++i, ++it) { + if (it == simulcast_rtp_rtcp_.end()) { + return -1; + } + } + *ssrc = (*it)->SSRC(); return 0; } diff --git a/src/video_engine/vie_channel.h b/src/video_engine/vie_channel.h index 062eb0114..bab5a8280 100644 --- a/src/video_engine/vie_channel.h +++ b/src/video_engine/vie_channel.h @@ -118,8 +118,8 @@ class ViEChannel const StreamType usage, const unsigned char simulcast_idx); - // Gets SSRC for outgoing stream. - WebRtc_Word32 GetLocalSSRC(uint32_t* ssrc); + // Gets SSRC for outgoing stream number |idx|. + WebRtc_Word32 GetLocalSSRC(uint8_t idx, unsigned int* ssrc); // Gets SSRC for the incoming stream. WebRtc_Word32 GetRemoteSSRC(uint32_t* ssrc); diff --git a/src/video_engine/vie_channel_manager.cc b/src/video_engine/vie_channel_manager.cc index fe0964b9e..ad181386c 100644 --- a/src/video_engine/vie_channel_manager.cc +++ b/src/video_engine/vie_channel_manager.cc @@ -121,8 +121,12 @@ int ViEChannelManager::CreateChannel(int* channel_id) { // Add ViEEncoder to EncoderFeedBackObserver. unsigned int ssrc = 0; - channel_map_[new_channel_id]->GetLocalSSRC(&ssrc); + int idx = 0; + channel_map_[new_channel_id]->GetLocalSSRC(idx, &ssrc); encoder_state_feedback->AddEncoder(ssrc, vie_encoder); + std::list ssrcs; + ssrcs.push_back(ssrc); + vie_encoder->SetSsrcs(ssrcs); *channel_id = new_channel_id; group->AddChannel(*channel_id); @@ -170,7 +174,8 @@ int ViEChannelManager::CreateChannel(int* channel_id, } // Register the ViEEncoder to get key frame requests for this channel. unsigned int ssrc = 0; - channel_map_[new_channel_id]->GetLocalSSRC(&ssrc); + int stream_idx = 0; + channel_map_[new_channel_id]->GetLocalSSRC(stream_idx, &ssrc); encoder_state_feedback->AddEncoder(ssrc, vie_encoder); } else { vie_encoder = ViEEncoderPtr(original_channel); @@ -224,14 +229,12 @@ int ViEChannelManager::DeleteChannel(int channel_id) { group = FindGroup(channel_id); group->SetChannelRembStatus(channel_id, false, false, vie_channel, vie_encoder); + group->GetEncoderStateFeedback()->RemoveEncoder(vie_encoder); + unsigned int remote_ssrc = 0; vie_channel->GetRemoteSSRC(&remote_ssrc); group->RemoveChannel(channel_id, remote_ssrc); - unsigned int local_ssrc = 0; - vie_channel->GetLocalSSRC(&local_ssrc); - group->GetEncoderStateFeedback()->RemoveEncoder(local_ssrc); - // Check if other channels are using the same encoder. if (ChannelUsingViEEncoder(channel_id)) { vie_encoder = NULL; @@ -370,6 +373,24 @@ bool ViEChannelManager::SetBandwidthEstimationMode( return true; } +void ViEChannelManager::UpdateSsrcs(int channel_id, + const std::list& ssrcs) { + CriticalSectionScoped cs(*channel_id_critsect_); + ChannelGroup* channel_group = FindGroup(channel_id); + if (channel_group == NULL) { + return; + } + ViEEncoder* encoder = ViEEncoderPtr(channel_id); + assert(encoder); + + EncoderStateFeedback* encoder_state_feedback = + channel_group->GetEncoderStateFeedback(); + for (std::list::const_iterator it = ssrcs.begin(); + it != ssrcs.end(); ++it) { + encoder_state_feedback->AddEncoder(*it, encoder); + } +} + bool ViEChannelManager::CreateChannelObject( int channel_id, ViEEncoder* vie_encoder, diff --git a/src/video_engine/vie_channel_manager.h b/src/video_engine/vie_channel_manager.h index 24097513f..04bd37aa8 100644 --- a/src/video_engine/vie_channel_manager.h +++ b/src/video_engine/vie_channel_manager.h @@ -79,6 +79,10 @@ class ViEChannelManager: private ViEManagerBase { // adding a channel. bool SetBandwidthEstimationMode(BandwidthEstimationMode mode); + // Updates the SSRCs for a channel. If one of the SSRCs already is registered, + // it will simply be ignored and no error is returned. + void UpdateSsrcs(int channel_id, const std::list& ssrcs); + private: // Creates a channel object connected to |vie_encoder|. Assumed to be called // protected. diff --git a/src/video_engine/vie_codec_impl.cc b/src/video_engine/vie_codec_impl.cc index dd4d37da2..7518d175d 100644 --- a/src/video_engine/vie_codec_impl.cc +++ b/src/video_engine/vie_codec_impl.cc @@ -10,6 +10,8 @@ #include "video_engine/vie_codec_impl.h" +#include + #include "engine_configurations.h" // NOLINT #include "modules/video_coding/main/interface/video_coding.h" #include "system_wrappers/interface/trace.h" @@ -238,6 +240,32 @@ int ViECodecImpl::SetSendCodec(const int video_channel, } } + // TODO(mflodman) Break out this part in GetLocalSsrcListi(). + // Update all SSRCs to ViEEncoder. + std::list ssrcs; + if (video_codec_internal.numberOfSimulcastStreams == 0) { + unsigned int ssrc = 0; + if (vie_channel->GetLocalSSRC(0, &ssrc) != 0) { + WEBRTC_TRACE(kTraceError, kTraceVideo, + ViEId(shared_data_->instance_id(), video_channel), + "%s: Could not get ssrc", __FUNCTION__); + } + ssrcs.push_back(ssrc); + } else { + for (int idx = 0; idx < video_codec_internal.numberOfSimulcastStreams; + ++idx) { + unsigned int ssrc = 0; + if (vie_channel->GetLocalSSRC(idx, &ssrc) != 0) { + WEBRTC_TRACE(kTraceError, kTraceVideo, + ViEId(shared_data_->instance_id(), video_channel), + "%s: Could not get ssrc for idx %d", __FUNCTION__, idx); + } + ssrcs.push_back(ssrc); + } + } + vie_encoder->SetSsrcs(ssrcs); + shared_data_->channel_manager()->UpdateSsrcs(video_channel, ssrcs); + // Update the protection mode, we might be switching NACK/FEC. vie_encoder->UpdateProtectionMethod(); diff --git a/src/video_engine/vie_encoder.cc b/src/video_engine/vie_encoder.cc index 83fdcf541..a2058b848 100644 --- a/src/video_engine/vie_encoder.cc +++ b/src/video_engine/vie_encoder.cc @@ -72,7 +72,6 @@ ViEEncoder::ViEEncoder(WebRtc_Word32 engine_id, data_cs_(CriticalSectionWrapper::CreateCriticalSection()), bitrate_controller_(bitrate_controller), paused_(false), - time_last_intra_request_ms_(0), channels_dropping_delta_frames_(0), drop_next_frame_(false), fec_enabled_(false), @@ -806,23 +805,79 @@ void ViEEncoder::OnReceivedRPSI(uint32_t /*ssrc*/, has_received_rpsi_ = true; } -void ViEEncoder::OnReceivedIntraFrameRequest(uint32_t /*ssrc*/) { +void ViEEncoder::OnReceivedIntraFrameRequest(uint32_t ssrc) { // Key frame request from remote side, signal to VCM. WEBRTC_TRACE(webrtc::kTraceStateInfo, webrtc::kTraceVideo, ViEId(engine_id_, channel_id_), "%s", __FUNCTION__); - WebRtc_Word64 now = TickTime::MillisecondTimestamp(); - if (time_last_intra_request_ms_ + kViEMinKeyRequestIntervalMs > now) { - WEBRTC_TRACE(webrtc::kTraceStream, webrtc::kTraceVideo, - ViEId(engine_id_, channel_id_), - "%s: Not not encoding new intra due to timing", __FUNCTION__); - return; + int idx = 0; + { + CriticalSectionScoped cs(data_cs_.get()); + std::map::iterator stream_it = ssrc_streams_.find(ssrc); + if (stream_it == ssrc_streams_.end()) { + assert(false); + return; + } + std::map::iterator time_it = + time_last_intra_request_ms_.find(ssrc); + if (time_it == time_last_intra_request_ms_.end()) { + time_last_intra_request_ms_[ssrc] = 0; + } + + WebRtc_Word64 now = TickTime::MillisecondTimestamp(); + if (time_last_intra_request_ms_[ssrc] + kViEMinKeyRequestIntervalMs > now) { + WEBRTC_TRACE(webrtc::kTraceStream, webrtc::kTraceVideo, + ViEId(engine_id_, channel_id_), + "%s: Not encoding new intra due to timing", __FUNCTION__); + return; + } + time_last_intra_request_ms_[ssrc] = now; + idx = stream_it->second; } - vcm_.IntraFrameRequest(0); - time_last_intra_request_ms_ = now; + // Release the critsect before triggering key frame. + vcm_.IntraFrameRequest(idx); } void ViEEncoder::OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc) { + CriticalSectionScoped cs(data_cs_.get()); + std::map::iterator it = ssrc_streams_.find(old_ssrc); + if (it == ssrc_streams_.end()) { + return; + } + + ssrc_streams_[new_ssrc] = it->second; + ssrc_streams_.erase(it); + + std::map::iterator time_it = + time_last_intra_request_ms_.find(old_ssrc); + int64_t last_intra_request_ms = 0; + if (time_it != time_last_intra_request_ms_.end()) { + last_intra_request_ms = time_it->second; + time_last_intra_request_ms_.erase(time_it); + } + time_last_intra_request_ms_[new_ssrc] = last_intra_request_ms; +} + +bool ViEEncoder::SetSsrcs(const std::list& ssrcs) { + VideoCodec codec; + if (vcm_.SendCodec(&codec) != 0) + return false; + + if (codec.numberOfSimulcastStreams > 0 && + ssrcs.size() != codec.numberOfSimulcastStreams) { + return -1; + } + + CriticalSectionScoped cs(data_cs_.get()); + ssrc_streams_.clear(); + time_last_intra_request_ms_.clear(); + int idx = 0; + for (std::list::const_iterator it = ssrcs.begin(); + it != ssrcs.end(); ++it, ++idx) { + unsigned int ssrc = *it; + ssrc_streams_[ssrc] = idx; + } + return 0; } // Called from ViEBitrateObserver. diff --git a/src/video_engine/vie_encoder.h b/src/video_engine/vie_encoder.h index d68706c61..dc74f3339 100644 --- a/src/video_engine/vie_encoder.h +++ b/src/video_engine/vie_encoder.h @@ -11,6 +11,9 @@ #ifndef WEBRTC_VIDEO_ENGINE_VIE_ENCODER_H_ #define WEBRTC_VIDEO_ENGINE_VIE_ENCODER_H_ +#include +#include + #include "common_types.h" // NOLINT #include "typedefs.h" //NOLINT #include "modules/bitrate_controller/include/bitrate_controller.h" @@ -134,6 +137,9 @@ class ViEEncoder virtual void OnReceivedRPSI(uint32_t ssrc, uint64_t picture_id); virtual void OnLocalSsrcChanged(uint32_t old_ssrc, uint32_t new_ssrc); + // Sets SSRCs for all streams. + bool SetSsrcs(const std::list& ssrcs); + // Effect filter. WebRtc_Word32 RegisterEffectFilter(ViEEffectFilter* effect_filter); @@ -167,7 +173,7 @@ class ViEEncoder BitrateController* bitrate_controller_; bool paused_; - WebRtc_Word64 time_last_intra_request_ms_; + std::map time_last_intra_request_ms_; WebRtc_Word32 channels_dropping_delta_frames_; bool drop_next_frame_; @@ -182,6 +188,7 @@ class ViEEncoder WebRtc_UWord8 picture_id_sli_; bool has_received_rpsi_; WebRtc_UWord64 picture_id_rpsi_; + std::map ssrc_streams_; ViEFileRecorder file_recorder_; diff --git a/src/video_engine/vie_rtp_rtcp_impl.cc b/src/video_engine/vie_rtp_rtcp_impl.cc index c716e778d..0c047d7ad 100644 --- a/src/video_engine/vie_rtp_rtcp_impl.cc +++ b/src/video_engine/vie_rtp_rtcp_impl.cc @@ -183,7 +183,8 @@ int ViERTP_RTCPImpl::GetLocalSSRC(const int video_channel, shared_data_->SetLastError(kViERtpRtcpInvalidChannelId); return -1; } - if (vie_channel->GetLocalSSRC(&SSRC) != 0) { + uint8_t idx = 0; + if (vie_channel->GetLocalSSRC(idx, &SSRC) != 0) { shared_data_->SetLastError(kViERtpRtcpUnknownError); return -1; }