Reland of Split IncomingVideoStream into two implementations, with smoothing and without. (patchset #1 id:1 of https://codereview.webrtc.org/2084873002/ )
Reason for revert:
Reverting the revert. This change is not related to the failure on the Windows FYI bots. The cause of the failure has been reverted in Chromium:
https://codereview.chromium.org/2081653004/
Original issue's description:
> Revert of Split IncomingVideoStream into two implementations, with smoothing and without. (patchset #5 id:340001 of https://codereview.webrtc.org/2078873002/ )
>
> Reason for revert:
> Breaks chromium.webrtc.fyi
>
> https://uberchromegw.corp.google.com/i/chromium.webrtc.fyi/builders/Win7%20Tester/builds/4719
> https://uberchromegw.corp.google.com/i/chromium.webrtc.fyi/builders/Win10%20Tester/builds/3120
>
> Original issue's description:
> > Reland of IncomingVideoStream refactoring.
> > This reland does not contain the non-smoothing part of the original implementation. Instead, when smoothing is turned off, frame callbacks run on the decoder thread, as they did before. This code path is used in Chrome. As far as Chrome goes, the difference now is that there won't be an instance of IncomingVideoStream in between the decoder and the callback (i.e. fewer locks). Other than that, no change for Chrome.
> >
> > Original issue's description (with non-smoothing references removed):
> >
> > Split IncomingVideoStream into two implementations, with smoothing and without.
> >
> > * Added TODOs and documentation for VideoReceiveStream::OnFrame, where we today grab 6 locks.
> >
> > * Removed the Start/Stop methods from the IncomingVideoStream implementations. Now, when an instance is created, it should be considered to be "running" and when it is deleted, it's "not running". This saves on resources and also reduces the amount of locking required and I could remove one critical section altogether.
> >
> > * Changed the VideoStreamDecoder class to not depend on IncomingVideoStream but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface. This means that any implementation of that interface can be used and the decoder can be made to just use the 'renderer' from the config. Once we do that, we can decouple the IncomingVideoStream implementations from the decoder and VideoReceiveStream implementations and leave it up to the application for how to do smoothing. The app can choose to use the Incoming* classes or roll its own (which may be preferable since applications often have their own scheduling mechanisms).
> >
> > * The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in VideoReceiveStream and not all of the lifetime of VideoReceiveStream.
> >
> > * Fixed VideoStreamDecoder to unregister callbacks in the dtor that were registered in the ctor. (this was open to a use-after-free regression)
> >
> > * Delay and callback pointers are now passed via the ctors to the IncomingVideoStream classes. The thread primitives in the IncomingVideoStream classes are also constructed/destructed at the same time as the owning object, which allowed me to remove one more lock.
> >
> > * Removed code in the VideoStreamDecoder that could overwrite the VideoReceiveStream render delay with a fixed value of 10ms on construction. This wasn't a problem with the previous implementation (it would be now though) but seemed to me like the wrong place to be setting that value.
> >
> > * Made the render delay value in VideoRenderFrames, const.
> >
> > BUG=chromium:620232
> > R=mflodman@webrtc.org, nisse@webrtc.org
> >
> > Committed: https://crrev.com/884c336c345d988974c2a69cea402b0fb8b07a63
> > Cr-Commit-Position: refs/heads/master@{#13219}
>
> TBR=nisse@webrtc.org,philipel@webrtc.org,mflodman@webrtc.org,tommi@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=chromium:620232
>
> Committed: https://crrev.com/a536bfe70de38fe877245317a7f0b00bcf69cbd0
> Cr-Commit-Position: refs/heads/master@{#13229}
TBR=nisse@webrtc.org,philipel@webrtc.org,mflodman@webrtc.org,sakal@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:620232
Review-Url: https://codereview.webrtc.org/2089613002
Cr-Commit-Position: refs/heads/master@{#13230}
diff --git a/webrtc/common_video/include/incoming_video_stream.h b/webrtc/common_video/include/incoming_video_stream.h
index f96a23d..b551d45 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<VideoFrame> {
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<VideoFrame>* render_object);
-
- // Start/Stop.
- int32_t Start();
- int32_t Stop();
-
- int32_t SetExpectedRenderDelay(int32_t delay_ms);
+ IncomingVideoStream(int32_t delay_ms,
+ rtc::VideoSinkInterface<VideoFrame>* callback);
+ ~IncomingVideoStream() override;
protected:
static bool IncomingVideoStreamThreadFun(void* obj);
@@ -49,23 +38,18 @@
enum { kEventMaxWaitTimeMs = 100 };
enum { kFrameRatePeriodMs = 1000 };
- void DeliverFrame(const VideoFrame& video_frame);
+ void OnFrame(const VideoFrame& video_frame) override;
- const bool disable_prerenderer_smoothing_;
- // Critsects in allowed to enter order.
- rtc::CriticalSection stream_critsect_;
- rtc::CriticalSection thread_critsect_;
+ rtc::ThreadChecker main_thread_checker_;
+ rtc::ThreadChecker render_thread_checker_;
+ rtc::ThreadChecker decoder_thread_checker_;
+
rtc::CriticalSection buffer_critsect_;
- // TODO(pbos): Make plain member and stop resetting this thread, just
- // start/stoping it is enough.
- std::unique_ptr<rtc::PlatformThread> incoming_render_thread_
- GUARDED_BY(thread_critsect_);
+ rtc::PlatformThread incoming_render_thread_;
std::unique_ptr<EventTimerWrapper> deliver_buffer_event_;
- bool running_ GUARDED_BY(stream_critsect_);
- rtc::VideoSinkInterface<VideoFrame>* external_callback_
- GUARDED_BY(thread_critsect_);
- const std::unique_ptr<VideoRenderFrames> render_buffers_
+ rtc::VideoSinkInterface<VideoFrame>* const external_callback_;
+ std::unique_ptr<VideoRenderFrames> 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 a5e7ba7..8deca0f 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 <assert.h>
-
-#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<VideoFrame>* 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<VideoFrame>* 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<IncomingVideoStream*>(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<VideoFrame> 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 @@
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 62b317d..b818512 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 @@
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 @@
return time_to_release < 0 ? 0u : static_cast<uint32_t>(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 acd9558..26a7ef5 100644
--- a/webrtc/common_video/video_render_frames.h
+++ b/webrtc/common_video/video_render_frames.h
@@ -23,7 +23,8 @@
// 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 @@
// Get a frame for rendering, or false if it's not time to render.
rtc::Optional<VideoFrame> 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 @@
std::list<VideoFrame> 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 df7677b..f8b20b0 100644
--- a/webrtc/media/base/videosinkinterface.h
+++ b/webrtc/media/base/videosinkinterface.h
@@ -19,9 +19,9 @@
template <typename VideoFrameT>
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 59cbf5c..21c54d1 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 @@
++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 @@
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 fb7cc49..43b66c0 100644
--- a/webrtc/video/end_to_end_tests.cc
+++ b/webrtc/video/end_to_end_tests.cc
@@ -2027,7 +2027,8 @@
void EndToEndTest::VerifyHistogramStats(bool use_rtx,
bool use_red,
bool screenshare) {
- class StatsObserver : public test::EndToEndTest {
+ class StatsObserver : public test::EndToEndTest,
+ public rtc::VideoSinkInterface<VideoFrame> {
public:
StatsObserver(bool use_rtx, bool use_red, bool screenshare)
: EndToEndTest(kLongTimeoutMs),
@@ -2043,6 +2044,8 @@
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 @@
// 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, GetStats) {
static const int kStartBitrateBps = 3000000;
static const int kExpectedRenderDelayMs = 20;
+
+ class ReceiveStreamRenderer : public rtc::VideoSinkInterface<VideoFrame> {
+ public:
+ ReceiveStreamRenderer() {}
+
+ private:
+ void OnFrame(const VideoFrame& video_frame) override {}
+ };
+
class StatsObserver : public test::EndToEndTest,
public rtc::VideoSinkInterface<VideoFrame> {
public:
@@ -2691,6 +2704,7 @@
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 @@
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 9909e29..ad9beb3 100644
--- a/webrtc/video/video_receive_stream.cc
+++ b/webrtc/video/video_receive_stream.cc
@@ -144,6 +144,7 @@
} // namespace
namespace internal {
+
VideoReceiveStream::VideoReceiveStream(
int num_cpu_cores,
CongestionController* congestion_controller,
@@ -160,7 +161,6 @@
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 @@
&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 @@
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<int> decoder_payload_types;
for (const Decoder& decoder : config_.decoders) {
@@ -210,8 +199,6 @@
}
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 @@
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 @@
if (decode_thread_.IsRunning())
return;
transport_adapter_.Enable();
- incoming_video_stream_.Start();
+ rtc::VideoSinkInterface<VideoFrame>* 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::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 @@
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 3f6d55a..d37aece 100644
--- a/webrtc/video/video_receive_stream.h
+++ b/webrtc/video/video_receive_stream.h
@@ -96,10 +96,10 @@
CallStats* const call_stats_;
vcm::VideoReceiver video_receiver_;
- IncomingVideoStream incoming_video_stream_;
+ std::unique_ptr<rtc::VideoSinkInterface<VideoFrame>> incoming_video_stream_;
ReceiveStatisticsProxy stats_proxy_;
RtpStreamReceiver rtp_stream_receiver_;
- VideoStreamDecoder video_stream_decoder_;
+ std::unique_ptr<VideoStreamDecoder> video_stream_decoder_;
ViESyncModule vie_sync_;
std::unique_ptr<IvfFileWriter> ivf_writer_;
diff --git a/webrtc/video/video_stream_decoder.cc b/webrtc/video/video_stream_decoder.cc
index 4caa9de..6797353 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 @@
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<VideoFrame>* incoming_video_stream,
I420FrameCallback* pre_render_callback)
: video_receiver_(video_receiver),
receive_stats_callback_(receive_statistics_proxy),
@@ -51,16 +50,10 @@
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 @@
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 @@
}
}
- 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 24a0ea3..11f0b03 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 Config;
class EncodedImageCallback;
class I420FrameCallback;
-class IncomingVideoStream;
class ReceiveStatisticsProxy;
class VideoRenderCallback;
class VoEVideoSync;
@@ -58,7 +58,7 @@
bool enable_nack,
bool enable_fec,
ReceiveStatisticsProxy* receive_statistics_proxy,
- IncomingVideoStream* incoming_video_stream,
+ rtc::VideoSinkInterface<VideoFrame>* incoming_video_stream,
I420FrameCallback* pre_render_callback);
~VideoStreamDecoder();
@@ -89,18 +89,16 @@
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<VideoFrame>* 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_);
