diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h index b469553f56..3a5591782f 100644 --- a/call/video_receive_stream.h +++ b/call/video_receive_stream.h @@ -193,17 +193,16 @@ class VideoReceiveStream { MediaTransportInterface* media_transport = nullptr; - // Must not be 'nullptr' when the stream is started. + // Must always be set. rtc::VideoSinkInterface* renderer = nullptr; // Expected delay needed by the renderer, i.e. the frame will be delivered // this many milliseconds, if possible, earlier than the ideal render time. - // Only valid if 'renderer' is set. int render_delay_ms = 10; - // If set, pass frames on to the renderer as soon as they are + // If false, pass frames on to the renderer as soon as they are // available. - bool disable_prerenderer_smoothing = false; + bool enable_prerenderer_smoothing = true; // Identifier for an A/V synchronization group. Empty string to disable. // TODO(pbos): Synchronize streams in a sync group, not just video streams diff --git a/media/base/media_config.h b/media/base/media_config.h index eb5d43b315..be314a8aa3 100644 --- a/media/base/media_config.h +++ b/media/base/media_config.h @@ -33,21 +33,15 @@ struct MediaConfig { // to VideoSendStream::Config::suspend_below_min_bitrate. bool suspend_below_min_bitrate = false; - // Set to true if the renderer has an algorithm of frame selection. - // If the value is true, then WebRTC will hand over a frame as soon as - // possible without delay, and rendering smoothness is completely the duty - // of the renderer; - // If the value is false, then WebRTC is responsible to delay frame release - // in order to increase rendering smoothness. - // - // This flag comes from PeerConnection's RtcConfiguration, but is - // currently only set by the command line flag - // 'disable-rtc-smoothness-algorithm'. - // WebRtcVideoChannel::AddRecvStream copies it to the created - // WebRtcVideoReceiveStream, where it is returned by the - // SmoothsRenderedFrames method. This method is used by the - // VideoReceiveStream, where the value is passed on to the - // IncomingVideoStream constructor. + // Enable buffering and playout timing smoothing of decoded frames. + // If set to true, then WebRTC will buffer and potentially drop decoded + // frames in order to keep a smooth rendering. + // If set to false, then WebRTC will hand over the frame from the decoder + // to the renderer as soon as possible, meaning that the renderer is + // responsible for smooth rendering. + // Note that even if this flag is set to false, dropping of frames can + // still happen pre-decode, e.g., dropping of higher temporal layers. + // This flag comes from the PeerConnection RtcConfiguration. bool enable_prerenderer_smoothing = true; // Enables periodic bandwidth probing in application-limited region. diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 7c7eab2347..f26e5dd654 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -1184,9 +1184,8 @@ bool WebRtcVideoChannel::AddRecvStream(const StreamParams& sp, ConfigureReceiverRtp(&config, &flexfec_config, sp); config.crypto_options = crypto_options_; - // TODO(nisse): Rename config variable to avoid negation. - config.disable_prerenderer_smoothing = - !video_config_.enable_prerenderer_smoothing; + config.enable_prerenderer_smoothing = + video_config_.enable_prerenderer_smoothing; if (!sp.stream_ids().empty()) { config.sync_group = sp.stream_ids()[0]; } diff --git a/video/end_to_end_tests/retransmission_tests.cc b/video/end_to_end_tests/retransmission_tests.cc index 19fb8b5b56..7d31aed96b 100644 --- a/video/end_to_end_tests/retransmission_tests.cc +++ b/video/end_to_end_tests/retransmission_tests.cc @@ -316,6 +316,7 @@ TEST_F(RetransmissionEndToEndTest, ReceivesPliAndRecoversWithNack) { TEST_F(RetransmissionEndToEndTest, ReceivesPliAndRecoversWithoutNack) { ReceivesPliAndRecovers(0); } + // This test drops second RTP packet with a marker bit set, makes sure it's // retransmitted and renders. Retransmission SSRCs are also checked. void RetransmissionEndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, @@ -399,7 +400,8 @@ void RetransmissionEndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, RTC_DCHECK(!orig_renderer_); orig_renderer_ = (*receive_configs)[0].renderer; RTC_DCHECK(orig_renderer_); - (*receive_configs)[0].disable_prerenderer_smoothing = true; + // To avoid post-decode frame dropping, disable the prerender buffer. + (*receive_configs)[0].enable_prerenderer_smoothing = false; (*receive_configs)[0].renderer = this; (*receive_configs)[0].rtp.nack.rtp_history_ms = kNackRtpHistoryMs; diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index 1875771c37..836ad94ebb 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -198,6 +198,7 @@ VideoReceiveStream::VideoReceiveStream( rtp_stream_sync_(this) { RTC_LOG(LS_INFO) << "VideoReceiveStream: " << config_.ToString(); + RTC_DCHECK(config_.renderer); RTC_DCHECK(process_thread_); RTC_DCHECK(call_stats_); @@ -269,11 +270,13 @@ void VideoReceiveStream::SetSync(Syncable* audio_syncable) { void VideoReceiveStream::Start() { RTC_DCHECK_CALLED_SEQUENTIALLY(&worker_sequence_checker_); - if (decode_thread_.IsRunning()) - return; - bool protected_by_fec = config_.rtp.protected_by_flexfec || - rtp_video_stream_receiver_.IsUlpfecEnabled(); + if (decode_thread_.IsRunning()) { + return; + } + + const bool protected_by_fec = config_.rtp.protected_by_flexfec || + rtp_video_stream_receiver_.IsUlpfecEnabled(); frame_buffer_->Start(); @@ -284,16 +287,13 @@ void VideoReceiveStream::Start() { transport_adapter_.Enable(); 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(); - } + if (config_.enable_prerenderer_smoothing) { + incoming_video_stream_.reset( + new IncomingVideoStream(config_.render_delay_ms, this)); + renderer = incoming_video_stream_.get(); + } else { + renderer = this; } - RTC_DCHECK(renderer != nullptr); for (const Decoder& decoder : config_.decoders) { std::unique_ptr video_decoder = @@ -329,6 +329,7 @@ void VideoReceiveStream::Start() { &codec, num_cpu_cores_, false)); } + RTC_DCHECK(renderer != nullptr); video_stream_decoder_.reset(new VideoStreamDecoder( &video_receiver_, &rtp_video_stream_receiver_, &rtp_video_stream_receiver_, @@ -408,7 +409,6 @@ void VideoReceiveStream::OnFrame(const VideoFrame& video_frame) { // TODO(tommi): OnSyncOffsetUpdated grabs a lock. stats_proxy_.OnSyncOffsetUpdated(sync_offset_ms, estimated_freq_khz); } - // config_.renderer must never be null if we're getting this callback. config_.renderer->OnFrame(video_frame); // TODO(tommi): OnRenderFrame grabs a lock too.