diff --git a/webrtc/common_video/include/incoming_video_stream.h b/webrtc/common_video/include/incoming_video_stream.h index f96a23dbea..b551d4575a 100644 --- a/webrtc/common_video/include/incoming_video_stream.h +++ b/webrtc/common_video/include/incoming_video_stream.h @@ -16,29 +16,18 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/platform_thread.h" #include "webrtc/base/thread_annotations.h" +#include "webrtc/base/thread_checker.h" #include "webrtc/common_video/video_render_frames.h" #include "webrtc/media/base/videosinkinterface.h" namespace webrtc { class EventTimerWrapper; - class IncomingVideoStream : public rtc::VideoSinkInterface { public: - explicit IncomingVideoStream(bool disable_prerenderer_smoothing); - ~IncomingVideoStream(); - - // Overrides VideoSinkInterface - void OnFrame(const VideoFrame& video_frame) override; - - // Callback for file recording, snapshot, ... - void SetExternalCallback(rtc::VideoSinkInterface* render_object); - - // Start/Stop. - int32_t Start(); - int32_t Stop(); - - int32_t SetExpectedRenderDelay(int32_t delay_ms); + IncomingVideoStream(int32_t delay_ms, + rtc::VideoSinkInterface* callback); + ~IncomingVideoStream() override; protected: static bool IncomingVideoStreamThreadFun(void* obj); @@ -49,23 +38,18 @@ class IncomingVideoStream : public rtc::VideoSinkInterface { enum { kEventMaxWaitTimeMs = 100 }; enum { kFrameRatePeriodMs = 1000 }; - void DeliverFrame(const VideoFrame& video_frame); + void OnFrame(const VideoFrame& video_frame) override; + + rtc::ThreadChecker main_thread_checker_; + rtc::ThreadChecker render_thread_checker_; + rtc::ThreadChecker decoder_thread_checker_; - const bool disable_prerenderer_smoothing_; - // Critsects in allowed to enter order. - rtc::CriticalSection stream_critsect_; - rtc::CriticalSection thread_critsect_; rtc::CriticalSection buffer_critsect_; - // TODO(pbos): Make plain member and stop resetting this thread, just - // start/stoping it is enough. - std::unique_ptr incoming_render_thread_ - GUARDED_BY(thread_critsect_); + rtc::PlatformThread incoming_render_thread_; std::unique_ptr deliver_buffer_event_; - bool running_ GUARDED_BY(stream_critsect_); - rtc::VideoSinkInterface* external_callback_ - GUARDED_BY(thread_critsect_); - const std::unique_ptr render_buffers_ + rtc::VideoSinkInterface* const external_callback_; + std::unique_ptr render_buffers_ GUARDED_BY(buffer_critsect_); }; diff --git a/webrtc/common_video/incoming_video_stream.cc b/webrtc/common_video/incoming_video_stream.cc index a5e7ba755d..8deca0fc83 100644 --- a/webrtc/common_video/incoming_video_stream.cc +++ b/webrtc/common_video/incoming_video_stream.cc @@ -10,9 +10,6 @@ #include "webrtc/common_video/include/incoming_video_stream.h" -#include - -#include "webrtc/base/platform_thread.h" #include "webrtc/base/timeutils.h" #include "webrtc/common_video/video_render_frames.h" #include "webrtc/system_wrappers/include/critical_section_wrapper.h" @@ -20,122 +17,65 @@ namespace webrtc { -IncomingVideoStream::IncomingVideoStream(bool disable_prerenderer_smoothing) - : disable_prerenderer_smoothing_(disable_prerenderer_smoothing), - incoming_render_thread_(), +IncomingVideoStream::IncomingVideoStream( + int32_t delay_ms, + rtc::VideoSinkInterface* callback) + : incoming_render_thread_(&IncomingVideoStreamThreadFun, + this, + "IncomingVideoStreamThread"), deliver_buffer_event_(EventTimerWrapper::Create()), - running_(false), - external_callback_(nullptr), - render_buffers_(new VideoRenderFrames()) {} + external_callback_(callback), + render_buffers_(new VideoRenderFrames(delay_ms)) { + RTC_DCHECK(external_callback_); + + render_thread_checker_.DetachFromThread(); + decoder_thread_checker_.DetachFromThread(); + + incoming_render_thread_.Start(); + incoming_render_thread_.SetPriority(rtc::kRealtimePriority); + deliver_buffer_event_->StartTimer(false, kEventStartupTimeMs); +} IncomingVideoStream::~IncomingVideoStream() { - Stop(); + RTC_DCHECK(main_thread_checker_.CalledOnValidThread()); + + { + rtc::CritScope cs(&buffer_critsect_); + render_buffers_.reset(); + } + + deliver_buffer_event_->Set(); + incoming_render_thread_.Stop(); + deliver_buffer_event_->StopTimer(); } void IncomingVideoStream::OnFrame(const VideoFrame& video_frame) { - rtc::CritScope csS(&stream_critsect_); - - if (!running_) { - return; - } + RTC_DCHECK_RUN_ON(&decoder_thread_checker_); // Hand over or insert frame. - if (disable_prerenderer_smoothing_) { - DeliverFrame(video_frame); - } else { - rtc::CritScope csB(&buffer_critsect_); - if (render_buffers_->AddFrame(video_frame) == 1) { - deliver_buffer_event_->Set(); - } + rtc::CritScope csB(&buffer_critsect_); + if (render_buffers_->AddFrame(video_frame) == 1) { + deliver_buffer_event_->Set(); } } -int32_t IncomingVideoStream::SetExpectedRenderDelay( - int32_t delay_ms) { - rtc::CritScope csS(&stream_critsect_); - if (running_) { - return -1; - } - rtc::CritScope cs(&buffer_critsect_); - return render_buffers_->SetRenderDelay(delay_ms); -} - -void IncomingVideoStream::SetExternalCallback( - rtc::VideoSinkInterface* external_callback) { - rtc::CritScope cs(&thread_critsect_); - external_callback_ = external_callback; -} - -int32_t IncomingVideoStream::Start() { - rtc::CritScope csS(&stream_critsect_); - if (running_) { - return 0; - } - - if (!disable_prerenderer_smoothing_) { - rtc::CritScope csT(&thread_critsect_); - assert(incoming_render_thread_ == NULL); - - incoming_render_thread_.reset(new rtc::PlatformThread( - IncomingVideoStreamThreadFun, this, "IncomingVideoStreamThread")); - if (!incoming_render_thread_) { - return -1; - } - - incoming_render_thread_->Start(); - incoming_render_thread_->SetPriority(rtc::kRealtimePriority); - deliver_buffer_event_->StartTimer(false, kEventStartupTimeMs); - } - - running_ = true; - return 0; -} - -int32_t IncomingVideoStream::Stop() { - rtc::CritScope cs_stream(&stream_critsect_); - - if (!running_) { - return 0; - } - - rtc::PlatformThread* thread = NULL; - { - rtc::CritScope cs_thread(&thread_critsect_); - if (incoming_render_thread_) { - // Setting the incoming render thread to NULL marks that we're performing - // a shutdown and will make IncomingVideoStreamProcess abort after wakeup. - thread = incoming_render_thread_.release(); - deliver_buffer_event_->StopTimer(); - // Set the event to allow the thread to wake up and shut down without - // waiting for a timeout. - deliver_buffer_event_->Set(); - } - } - if (thread) { - thread->Stop(); - delete thread; - } - running_ = false; - return 0; -} - bool IncomingVideoStream::IncomingVideoStreamThreadFun(void* obj) { return static_cast(obj)->IncomingVideoStreamProcess(); } bool IncomingVideoStream::IncomingVideoStreamProcess() { - if (kEventError != deliver_buffer_event_->Wait(kEventMaxWaitTimeMs)) { - rtc::CritScope cs(&thread_critsect_); - if (incoming_render_thread_ == NULL) { - // Terminating - return false; - } + RTC_DCHECK_RUN_ON(&render_thread_checker_); + if (kEventError != deliver_buffer_event_->Wait(kEventMaxWaitTimeMs)) { // Get a new frame to render and the time for the frame after this one. rtc::Optional frame_to_render; uint32_t wait_time; { rtc::CritScope cs(&buffer_critsect_); + if (!render_buffers_.get()) { + // Terminating + return false; + } frame_to_render = render_buffers_->FrameToRender(); wait_time = render_buffers_->TimeToNextFrameRelease(); } @@ -144,21 +84,14 @@ bool IncomingVideoStream::IncomingVideoStreamProcess() { if (wait_time > kEventMaxWaitTimeMs) { wait_time = kEventMaxWaitTimeMs; } + deliver_buffer_event_->StartTimer(false, wait_time); - if (frame_to_render) - DeliverFrame(*frame_to_render); + if (frame_to_render) { + external_callback_->OnFrame(*frame_to_render); + } } return true; } -void IncomingVideoStream::DeliverFrame(const VideoFrame& video_frame) { - rtc::CritScope cs(&thread_critsect_); - - // Send frame for rendering. - if (external_callback_) { - external_callback_->OnFrame(video_frame); - } -} - } // namespace webrtc diff --git a/webrtc/common_video/video_render_frames.cc b/webrtc/common_video/video_render_frames.cc index 62b317d966..b818512acb 100644 --- a/webrtc/common_video/video_render_frames.cc +++ b/webrtc/common_video/video_render_frames.cc @@ -17,14 +17,21 @@ #include "webrtc/system_wrappers/include/trace.h" namespace webrtc { +namespace { -const uint32_t KEventMaxWaitTimeMs = 200; +const uint32_t kEventMaxWaitTimeMs = 200; const uint32_t kMinRenderDelayMs = 10; const uint32_t kMaxRenderDelayMs = 500; -VideoRenderFrames::VideoRenderFrames() - : render_delay_ms_(10) { +uint32_t EnsureValidRenderDelay(uint32_t render_delay) { + return (render_delay < kMinRenderDelayMs || render_delay > kMaxRenderDelayMs) + ? kMinRenderDelayMs + : render_delay; } +} // namespace + +VideoRenderFrames::VideoRenderFrames(uint32_t render_delay_ms) + : render_delay_ms_(EnsureValidRenderDelay(render_delay_ms)) {} int32_t VideoRenderFrames::AddFrame(const VideoFrame& new_frame) { const int64_t time_now = rtc::TimeMillis(); @@ -63,14 +70,9 @@ rtc::Optional VideoRenderFrames::FrameToRender() { return render_frame; } -int32_t VideoRenderFrames::ReleaseAllFrames() { - incoming_frames_.clear(); - return 0; -} - uint32_t VideoRenderFrames::TimeToNextFrameRelease() { if (incoming_frames_.empty()) { - return KEventMaxWaitTimeMs; + return kEventMaxWaitTimeMs; } const int64_t time_to_release = incoming_frames_.front().render_time_ms() - render_delay_ms_ - @@ -78,18 +80,4 @@ uint32_t VideoRenderFrames::TimeToNextFrameRelease() { return time_to_release < 0 ? 0u : static_cast(time_to_release); } -int32_t VideoRenderFrames::SetRenderDelay( - const uint32_t render_delay) { - if (render_delay < kMinRenderDelayMs || - render_delay > kMaxRenderDelayMs) { - WEBRTC_TRACE(kTraceWarning, kTraceVideoRenderer, - -1, "%s(%d): Invalid argument.", __FUNCTION__, - render_delay); - return -1; - } - - render_delay_ms_ = render_delay; - return 0; -} - } // namespace webrtc diff --git a/webrtc/common_video/video_render_frames.h b/webrtc/common_video/video_render_frames.h index acd955887f..26a7ef5137 100644 --- a/webrtc/common_video/video_render_frames.h +++ b/webrtc/common_video/video_render_frames.h @@ -23,7 +23,8 @@ namespace webrtc { // Class definitions class VideoRenderFrames { public: - VideoRenderFrames(); + explicit VideoRenderFrames(uint32_t render_delay_ms); + VideoRenderFrames(const VideoRenderFrames&) = delete; // Add a frame to the render queue int32_t AddFrame(const VideoFrame& new_frame); @@ -31,15 +32,9 @@ class VideoRenderFrames { // Get a frame for rendering, or false if it's not time to render. rtc::Optional FrameToRender(); - // Releases all frames - int32_t ReleaseAllFrames(); - // Returns the number of ms to next frame to render uint32_t TimeToNextFrameRelease(); - // Sets estimates delay in renderer - int32_t SetRenderDelay(const uint32_t render_delay); - private: // 10 seconds for 30 fps. enum { KMaxNumberOfFrames = 300 }; @@ -52,7 +47,7 @@ class VideoRenderFrames { std::list incoming_frames_; // Estimated delay from a frame is released until it's rendered. - uint32_t render_delay_ms_; + const uint32_t render_delay_ms_; }; } // namespace webrtc diff --git a/webrtc/media/base/videosinkinterface.h b/webrtc/media/base/videosinkinterface.h index df7677bdaf..f8b20b0947 100644 --- a/webrtc/media/base/videosinkinterface.h +++ b/webrtc/media/base/videosinkinterface.h @@ -19,9 +19,9 @@ namespace rtc { template class VideoSinkInterface { public: - virtual void OnFrame(const VideoFrameT& frame) = 0; - protected: virtual ~VideoSinkInterface() {} + + virtual void OnFrame(const VideoFrameT& frame) = 0; }; } // namespace rtc diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc index 59cbf5c78a..21c54d1c63 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc @@ -136,8 +136,6 @@ RemoteBitrateEstimatorAbsSendTime::FindBestProbe( ++it) { if (it->send_mean_ms == 0 || it->recv_mean_ms == 0) continue; - int send_bitrate_bps = it->mean_size * 8 * 1000 / it->send_mean_ms; - int recv_bitrate_bps = it->mean_size * 8 * 1000 / it->recv_mean_ms; if (it->num_above_min_delta > it->count / 2 && (it->recv_mean_ms - it->send_mean_ms <= 2.0f && it->send_mean_ms - it->recv_mean_ms <= 5.0f)) { @@ -148,6 +146,8 @@ RemoteBitrateEstimatorAbsSendTime::FindBestProbe( best_it = it; } } else { + int send_bitrate_bps = it->mean_size * 8 * 1000 / it->send_mean_ms; + int recv_bitrate_bps = it->mean_size * 8 * 1000 / it->recv_mean_ms; LOG(LS_INFO) << "Probe failed, sent at " << send_bitrate_bps << " bps, received at " << recv_bitrate_bps << " bps. Mean send delta: " << it->send_mean_ms diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index fb7cc491d9..43b66c0861 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -2027,7 +2027,8 @@ TEST_F(EndToEndTest, VerifyNackStats) { void EndToEndTest::VerifyHistogramStats(bool use_rtx, bool use_red, bool screenshare) { - class StatsObserver : public test::EndToEndTest { + class StatsObserver : public test::EndToEndTest, + public rtc::VideoSinkInterface { public: StatsObserver(bool use_rtx, bool use_red, bool screenshare) : EndToEndTest(kLongTimeoutMs), @@ -2043,6 +2044,8 @@ void EndToEndTest::VerifyHistogramStats(bool use_rtx, start_runtime_ms_(-1) {} private: + void OnFrame(const VideoFrame& video_frame) override {} + Action OnSendRtp(const uint8_t* packet, size_t length) override { if (MinMetricRunTimePassed()) observation_complete_.Set(); @@ -2067,6 +2070,7 @@ void EndToEndTest::VerifyHistogramStats(bool use_rtx, // NACK send_config->rtp.nack.rtp_history_ms = kNackRtpHistoryMs; (*receive_configs)[0].rtp.nack.rtp_history_ms = kNackRtpHistoryMs; + (*receive_configs)[0].renderer = this; // FEC if (use_red_) { send_config->rtp.fec.ulpfec_payload_type = kUlpfecPayloadType; @@ -2491,6 +2495,15 @@ TEST_F(EndToEndTest, ReportsSetEncoderRates) { TEST_F(EndToEndTest, GetStats) { static const int kStartBitrateBps = 3000000; static const int kExpectedRenderDelayMs = 20; + + class ReceiveStreamRenderer : public rtc::VideoSinkInterface { + public: + ReceiveStreamRenderer() {} + + private: + void OnFrame(const VideoFrame& video_frame) override {} + }; + class StatsObserver : public test::EndToEndTest, public rtc::VideoSinkInterface { public: @@ -2691,6 +2704,7 @@ TEST_F(EndToEndTest, GetStats) { expected_receive_ssrcs_.push_back( (*receive_configs)[i].rtp.remote_ssrc); (*receive_configs)[i].render_delay_ms = kExpectedRenderDelayMs; + (*receive_configs)[i].renderer = &receive_stream_renderer_; } // Use a delayed encoder to make sure we see CpuOveruseMetrics stats that // are non-zero. @@ -2760,6 +2774,7 @@ TEST_F(EndToEndTest, GetStats) { std::string expected_cname_; rtc::Event check_stats_event_; + ReceiveStreamRenderer receive_stream_renderer_; } test; RunBaseTest(&test); diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 9909e29c43..ad9beb35e7 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -144,6 +144,7 @@ VideoCodec CreateDecoderVideoCodec(const VideoReceiveStream::Decoder& decoder) { } // namespace namespace internal { + VideoReceiveStream::VideoReceiveStream( int num_cpu_cores, CongestionController* congestion_controller, @@ -160,7 +161,6 @@ VideoReceiveStream::VideoReceiveStream( congestion_controller_(congestion_controller), call_stats_(call_stats), video_receiver_(clock_, nullptr, this, this, this), - incoming_video_stream_(config_.disable_prerenderer_smoothing), stats_proxy_(&config_, clock_), rtp_stream_receiver_(&video_receiver_, congestion_controller_->GetRemoteBitrateEstimator( @@ -173,14 +173,6 @@ VideoReceiveStream::VideoReceiveStream( &config_, &stats_proxy_, process_thread_), - video_stream_decoder_(&video_receiver_, - &rtp_stream_receiver_, - &rtp_stream_receiver_, - rtp_stream_receiver_.IsRetransmissionsEnabled(), - rtp_stream_receiver_.IsFecEnabled(), - &stats_proxy_, - &incoming_video_stream_, - config.pre_render_callback), vie_sync_(&video_receiver_) { LOG(LS_INFO) << "VideoReceiveStream: " << config_.ToString(); @@ -188,9 +180,6 @@ VideoReceiveStream::VideoReceiveStream( RTC_DCHECK(congestion_controller_); RTC_DCHECK(call_stats_); - // Register the channel to receive stats updates. - call_stats_->RegisterStatsObserver(&video_stream_decoder_); - RTC_DCHECK(!config_.decoders.empty()); std::set decoder_payload_types; for (const Decoder& decoder : config_.decoders) { @@ -210,8 +199,6 @@ VideoReceiveStream::VideoReceiveStream( } video_receiver_.SetRenderDelay(config.render_delay_ms); - incoming_video_stream_.SetExpectedRenderDelay(config.render_delay_ms); - incoming_video_stream_.SetExternalCallback(this); process_thread_->RegisterModule(&video_receiver_); process_thread_->RegisterModule(&vie_sync_); @@ -231,8 +218,6 @@ VideoReceiveStream::~VideoReceiveStream() { for (const Decoder& decoder : config_.decoders) video_receiver_.RegisterExternalDecoder(nullptr, decoder.payload_type); - call_stats_->DeregisterStatsObserver(&video_stream_decoder_); - congestion_controller_->GetRemoteBitrateEstimator(UseSendSideBwe(config_)) ->RemoveStream(rtp_stream_receiver_.GetRemoteSsrc()); } @@ -256,7 +241,24 @@ void VideoReceiveStream::Start() { if (decode_thread_.IsRunning()) return; transport_adapter_.Enable(); - incoming_video_stream_.Start(); + rtc::VideoSinkInterface* renderer = nullptr; + if (config_.renderer) { + if (config_.disable_prerenderer_smoothing) { + renderer = this; + } else { + incoming_video_stream_.reset( + new IncomingVideoStream(config_.render_delay_ms, this)); + renderer = incoming_video_stream_.get(); + } + } + + video_stream_decoder_.reset(new VideoStreamDecoder( + &video_receiver_, &rtp_stream_receiver_, &rtp_stream_receiver_, + rtp_stream_receiver_.IsRetransmissionsEnabled(), + rtp_stream_receiver_.IsFecEnabled(), &stats_proxy_, renderer, + config_.pre_render_callback)); + // Register the channel to receive stats updates. + call_stats_->RegisterStatsObserver(video_stream_decoder_.get()); // Start the decode thread decode_thread_.Start(); decode_thread_.SetPriority(rtc::kHighestPriority); @@ -264,10 +266,12 @@ void VideoReceiveStream::Start() { } void VideoReceiveStream::Stop() { - incoming_video_stream_.Stop(); rtp_stream_receiver_.StopReceive(); video_receiver_.TriggerDecoderShutdown(); decode_thread_.Stop(); + call_stats_->DeregisterStatsObserver(video_stream_decoder_.get()); + video_stream_decoder_.reset(); + incoming_video_stream_.reset(); transport_adapter_.Disable(); } @@ -289,16 +293,26 @@ VideoReceiveStream::Stats VideoReceiveStream::GetStats() const { return stats_proxy_.GetStats(); } +// TODO(tommi): This method grabs a lock 6 times. void VideoReceiveStream::OnFrame(const VideoFrame& video_frame) { + // TODO(tommi): OnDecodedFrame grabs a lock, incidentally the same lock + // that OnSyncOffsetUpdated() and OnRenderedFrame() below grab. stats_proxy_.OnDecodedFrame(); int64_t sync_offset_ms; - if (vie_sync_.GetStreamSyncOffsetInMs(video_frame, &sync_offset_ms)) + // TODO(tommi): GetStreamSyncOffsetInMs grabs three locks. One inside the + // function itself, another in GetChannel() and a third in + // GetPlayoutTimestamp. Seems excessive. Anyhow, I'm assuming the function + // succeeds most of the time, which leads to grabbing a fourth lock. + if (vie_sync_.GetStreamSyncOffsetInMs(video_frame, &sync_offset_ms)) { + // TODO(tommi): OnSyncOffsetUpdated grabs a lock. stats_proxy_.OnSyncOffsetUpdated(sync_offset_ms); + } - if (config_.renderer) - config_.renderer->OnFrame(video_frame); + // config_.renderer must never be null if we're getting this callback. + config_.renderer->OnFrame(video_frame); + // TODO(tommi): OnRenderFrame grabs a lock too. stats_proxy_.OnRenderedFrame(video_frame.width(), video_frame.height()); } diff --git a/webrtc/video/video_receive_stream.h b/webrtc/video/video_receive_stream.h index 3f6d55a0da..d37aece98c 100644 --- a/webrtc/video/video_receive_stream.h +++ b/webrtc/video/video_receive_stream.h @@ -96,10 +96,10 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, CallStats* const call_stats_; vcm::VideoReceiver video_receiver_; - IncomingVideoStream incoming_video_stream_; + std::unique_ptr> incoming_video_stream_; ReceiveStatisticsProxy stats_proxy_; RtpStreamReceiver rtp_stream_receiver_; - VideoStreamDecoder video_stream_decoder_; + std::unique_ptr video_stream_decoder_; ViESyncModule vie_sync_; std::unique_ptr ivf_writer_; diff --git a/webrtc/video/video_stream_decoder.cc b/webrtc/video/video_stream_decoder.cc index 4caa9dea03..6797353161 100644 --- a/webrtc/video/video_stream_decoder.cc +++ b/webrtc/video/video_stream_decoder.cc @@ -17,7 +17,6 @@ #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" #include "webrtc/common_video/include/frame_callback.h" -#include "webrtc/common_video/include/incoming_video_stream.h" #include "webrtc/modules/video_coding/video_coding_impl.h" #include "webrtc/modules/video_processing/include/video_processing.h" #include "webrtc/system_wrappers/include/metrics.h" @@ -32,9 +31,9 @@ VideoStreamDecoder::VideoStreamDecoder( VCMFrameTypeCallback* vcm_frame_type_callback, VCMPacketRequestCallback* vcm_packet_request_callback, bool enable_nack, - bool enable_fec, // TODO(philipel): Actually use this. + bool enable_fec, ReceiveStatisticsProxy* receive_statistics_proxy, - IncomingVideoStream* incoming_video_stream, + rtc::VideoSinkInterface* incoming_video_stream, I420FrameCallback* pre_render_callback) : video_receiver_(video_receiver), receive_stats_callback_(receive_statistics_proxy), @@ -51,16 +50,10 @@ VideoStreamDecoder::VideoStreamDecoder( video_receiver_->RegisterFrameTypeCallback(vcm_frame_type_callback); video_receiver_->RegisterReceiveStatisticsCallback(this); video_receiver_->RegisterDecoderTimingCallback(this); - static const int kDefaultRenderDelayMs = 10; - video_receiver_->SetRenderDelay(kDefaultRenderDelayMs); - VCMVideoProtection video_protection = kProtectionNone; - if (enable_nack) { - if (enable_fec) - video_protection = kProtectionNackFEC; - else - video_protection = kProtectionNack; - } + VCMVideoProtection video_protection = + enable_nack ? (enable_fec ? kProtectionNackFEC : kProtectionNack) + : kProtectionNone; VCMDecodeErrorMode decode_error_mode = enable_nack ? kNoErrors : kWithErrors; video_receiver_->SetVideoProtection(video_protection, true); @@ -70,7 +63,14 @@ VideoStreamDecoder::VideoStreamDecoder( video_receiver_->RegisterPacketRequestCallback(packet_request_callback); } -VideoStreamDecoder::~VideoStreamDecoder() {} +VideoStreamDecoder::~VideoStreamDecoder() { + // Unset all the callback pointers that we set in the ctor. + video_receiver_->RegisterPacketRequestCallback(nullptr); + video_receiver_->RegisterDecoderTimingCallback(nullptr); + video_receiver_->RegisterReceiveStatisticsCallback(nullptr); + video_receiver_->RegisterFrameTypeCallback(nullptr); + video_receiver_->RegisterReceiveCallback(nullptr); +} // Do not acquire the lock of |video_receiver_| in this function. Decode // callback won't necessarily be called from the decoding thread. The decoding @@ -84,7 +84,9 @@ int32_t VideoStreamDecoder::FrameToRender(VideoFrame& video_frame) { // NOLINT } } - incoming_video_stream_->OnFrame(video_frame); + if (incoming_video_stream_) + incoming_video_stream_->OnFrame(video_frame); + return 0; } diff --git a/webrtc/video/video_stream_decoder.h b/webrtc/video/video_stream_decoder.h index 24a0ea3449..11f0b03d60 100644 --- a/webrtc/video/video_stream_decoder.h +++ b/webrtc/video/video_stream_decoder.h @@ -19,6 +19,7 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/platform_thread.h" #include "webrtc/base/scoped_ref_ptr.h" +#include "webrtc/media/base/videosinkinterface.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/modules/video_coding/include/video_coding_defines.h" #include "webrtc/typedefs.h" @@ -31,7 +32,6 @@ class ChannelStatsObserver; class Config; class EncodedImageCallback; class I420FrameCallback; -class IncomingVideoStream; class ReceiveStatisticsProxy; class VideoRenderCallback; class VoEVideoSync; @@ -58,7 +58,7 @@ class VideoStreamDecoder : public VCMReceiveCallback, bool enable_nack, bool enable_fec, ReceiveStatisticsProxy* receive_statistics_proxy, - IncomingVideoStream* incoming_video_stream, + rtc::VideoSinkInterface* incoming_video_stream, I420FrameCallback* pre_render_callback); ~VideoStreamDecoder(); @@ -89,18 +89,16 @@ class VideoStreamDecoder : public VCMReceiveCallback, void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; private: - // Assumed to be protected. - void StartDecodeThread(); - void StopDecodeThread(); - // Used for all registered callbacks except rendering. rtc::CriticalSection crit_; vcm::VideoReceiver* const video_receiver_; ReceiveStatisticsProxy* const receive_stats_callback_; - IncomingVideoStream* const incoming_video_stream_; + rtc::VideoSinkInterface* const incoming_video_stream_; + // TODO(tommi): This callback is basically the same thing as the one above. + // We shouldn't need to support both. I420FrameCallback* const pre_render_callback_; int64_t last_rtt_ms_ GUARDED_BY(crit_);