diff --git a/api/video/video_frame.cc b/api/video/video_frame.cc index e91eedede8..7a71f43493 100644 --- a/api/video/video_frame.cc +++ b/api/video/video_frame.cc @@ -10,11 +10,55 @@ #include "api/video/video_frame.h" +#include + #include "rtc_base/checks.h" #include "rtc_base/time_utils.h" namespace webrtc { +void VideoFrame::UpdateRect::Union(const UpdateRect& other) { + if (other.IsEmpty()) + return; + if (IsEmpty()) { + *this = other; + return; + } + int right = std::max(offset_x + width, other.offset_x + other.width); + int bottom = std::max(offset_y + height, other.offset_y + other.height); + offset_x = std::min(offset_x, other.offset_x); + offset_y = std::min(offset_y, other.offset_y); + width = right - offset_x; + height = bottom - offset_y; + RTC_DCHECK_GT(width, 0); + RTC_DCHECK_GT(height, 0); +} + +void VideoFrame::UpdateRect::Intersect(const UpdateRect& other) { + if (other.IsEmpty() || IsEmpty()) { + MakeEmptyUpdate(); + return; + } + + int right = std::min(offset_x + width, other.offset_x + other.width); + int bottom = std::min(offset_y + height, other.offset_y + other.height); + offset_x = std::max(offset_x, other.offset_x); + offset_y = std::max(offset_y, other.offset_y); + width = right - offset_x; + height = bottom - offset_y; + if (width <= 0 || height <= 0) { + MakeEmptyUpdate(); + } +} + +void VideoFrame::UpdateRect::MakeEmptyUpdate() { + width = height = offset_x = offset_y = 0; +} + +bool VideoFrame::UpdateRect::IsEmpty() const { + return width == 0 && height == 0; +} + VideoFrame::Builder::Builder() = default; VideoFrame::Builder::~Builder() = default; diff --git a/api/video/video_frame.h b/api/video/video_frame.h index 126c3d05de..c7dd02a7b1 100644 --- a/api/video/video_frame.h +++ b/api/video/video_frame.h @@ -31,6 +31,17 @@ class RTC_EXPORT VideoFrame { int offset_y; int width; int height; + + // Makes this UpdateRect a bounding box of this and other rect. + void Union(const UpdateRect& other); + + // Makes this UpdateRect an intersection of this and other rect. + void Intersect(const UpdateRect& other); + + // Sets everything to 0, making this UpdateRect a zero-size (empty) update. + void MakeEmptyUpdate(); + + bool IsEmpty() const; }; // Preferred way of building VideoFrame objects. diff --git a/media/base/video_broadcaster.cc b/media/base/video_broadcaster.cc index a2009f21b3..06001acc06 100644 --- a/media/base/video_broadcaster.cc +++ b/media/base/video_broadcaster.cc @@ -28,6 +28,10 @@ void VideoBroadcaster::AddOrUpdateSink( const VideoSinkWants& wants) { RTC_DCHECK(sink != nullptr); rtc::CritScope cs(&sinks_and_wants_lock_); + if (!FindSinkPair(sink)) { + // |Sink| is a new sink, which didn't receive previous frame. + previous_frame_sent_to_all_sinks_ = false; + } VideoSourceBase::AddOrUpdateSink(sink, wants); UpdateWants(); } @@ -52,6 +56,7 @@ VideoSinkWants VideoBroadcaster::wants() const { void VideoBroadcaster::OnFrame(const webrtc::VideoFrame& frame) { rtc::CritScope cs(&sinks_and_wants_lock_); + bool current_frame_was_discarded = false; for (auto& sink_pair : sink_pairs()) { if (sink_pair.wants.rotation_applied && frame.rotation() != webrtc::kVideoRotation_0) { @@ -60,6 +65,8 @@ void VideoBroadcaster::OnFrame(const webrtc::VideoFrame& frame) { // with rotation still pending. Protect sinks that don't expect any // pending rotation. RTC_LOG(LS_VERBOSE) << "Discarding frame with unexpected rotation."; + sink_pair.sink->OnDiscardedFrame(); + current_frame_was_discarded = true; continue; } if (sink_pair.wants.black_frames) { @@ -72,10 +79,23 @@ void VideoBroadcaster::OnFrame(const webrtc::VideoFrame& frame) { .set_id(frame.id()) .build(); sink_pair.sink->OnFrame(black_frame); + } else if (!previous_frame_sent_to_all_sinks_) { + // Since last frame was not sent to some sinks, full update is needed. + // Update_rect is not set here for this reason. + webrtc::VideoFrame copy = + webrtc::VideoFrame::Builder() + .set_video_frame_buffer(frame.video_frame_buffer()) + .set_rotation(frame.rotation()) + .set_timestamp_us(frame.timestamp_us()) + .set_id(frame.id()) + .set_color_space(frame.color_space()) + .build(); + sink_pair.sink->OnFrame(copy); } else { sink_pair.sink->OnFrame(frame); } } + previous_frame_sent_to_all_sinks_ = !current_frame_was_discarded; } void VideoBroadcaster::OnDiscardedFrame() { diff --git a/media/base/video_broadcaster.h b/media/base/video_broadcaster.h index 20111bcac6..898ef2ac9a 100644 --- a/media/base/video_broadcaster.h +++ b/media/base/video_broadcaster.h @@ -60,6 +60,8 @@ class VideoBroadcaster : public VideoSourceBase, VideoSinkWants current_wants_ RTC_GUARDED_BY(sinks_and_wants_lock_); rtc::scoped_refptr black_frame_buffer_; + bool previous_frame_sent_to_all_sinks_ RTC_GUARDED_BY(sinks_and_wants_lock_) = + true; }; } // namespace rtc diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index 3d184bdf01..d5474405cf 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -418,6 +418,8 @@ int SimulcastEncoderAdapter::Encode( dst_buffer->StrideV(), dst_width, dst_height, libyuv::kFilterBilinear); + // UpdateRect is not propagated to lower simulcast layers currently. + // TODO(ilnik): Consider scaling UpdateRect together with the buffer. VideoFrame frame = VideoFrame::Builder() .set_video_frame_buffer(dst_buffer) .set_timestamp_rtp(input_image.timestamp()) diff --git a/modules/video_coding/video_sender.cc b/modules/video_coding/video_sender.cc index 5b3a657878..4493dd7c67 100644 --- a/modules/video_coding/video_sender.cc +++ b/modules/video_coding/video_sender.cc @@ -199,6 +199,10 @@ int32_t VideoSender::AddVideoFrame( RTC_LOG(LS_ERROR) << "Frame conversion failed, dropping frame."; return VCM_PARAMETER_ERROR; } + + // UpdatedRect is not propagated because buffer was converted, + // therefore we can't guarantee that pixels outside of UpdateRect didn't + // change comparing to the previous frame. converted_frame = VideoFrame::Builder() .set_video_frame_buffer(converted_buffer) .set_timestamp_rtp(converted_frame.timestamp()) diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index fb9dd8b8f1..8436dd8a3d 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -388,6 +388,7 @@ VideoStreamEncoder::VideoStreamEncoder( captured_frame_count_(0), dropped_frame_count_(0), pending_frame_post_time_us_(0), + accumulated_update_rect_{0, 0, 0, 0}, bitrate_observer_(nullptr), force_disable_frame_dropper_(false), input_framerate_(kFrameRateAvergingWindowSizeMs, 1000), @@ -758,6 +759,10 @@ void VideoStreamEncoder::OnFrame(const VideoFrame& video_frame) { << incoming_frame.ntp_time_ms() << " <= " << last_captured_timestamp_ << ") for incoming frame. Dropping."; + encoder_queue_.PostTask([this, incoming_frame]() { + RTC_DCHECK_RUN_ON(&encoder_queue_); + accumulated_update_rect_.Union(incoming_frame.update_rect()); + }); return; } @@ -790,6 +795,7 @@ void VideoStreamEncoder::OnFrame(const VideoFrame& video_frame) { ++dropped_frame_count_; encoder_stats_observer_->OnFrameDropped( VideoStreamEncoderObserver::DropReason::kEncoderQueue); + accumulated_update_rect_.Union(incoming_frame.update_rect()); } if (log_stats) { RTC_LOG(LS_INFO) << "Number of frames: captured " @@ -878,6 +884,9 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, << last_frame_info_->width << "x" << last_frame_info_->height << ", texture=" << last_frame_info_->is_texture << "."; + // Force full frame update, since resolution has changed. + accumulated_update_rect_ = + VideoFrame::UpdateRect{0, 0, video_frame.width(), video_frame.height()}; } // We have to create then encoder before the frame drop logic, @@ -905,6 +914,14 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, last_parameters_update_ms_.emplace(now_ms); } + // Because pending frame will be dropped in any case, we need to + // remember its updated region. + if (pending_frame_) { + encoder_stats_observer_->OnFrameDropped( + VideoStreamEncoderObserver::DropReason::kEncoderQueue); + accumulated_update_rect_.Union(pending_frame_->update_rect()); + } + if (DropDueToSize(video_frame.size())) { RTC_LOG(LS_INFO) << "Dropping frame. Too large for target bitrate."; int count = GetConstAdaptCounter().ResolutionCount(kQuality); @@ -921,6 +938,7 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, } else { // Ensure that any previously stored frame is dropped. pending_frame_.reset(); + accumulated_update_rect_.Union(video_frame.update_rect()); } return; } @@ -938,6 +956,7 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, // Ensure that any previously stored frame is dropped. pending_frame_.reset(); TraceFrameDropStart(); + accumulated_update_rect_.Union(video_frame.update_rect()); } return; } @@ -957,6 +976,7 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, << ", input frame rate " << framerate_fps; OnDroppedFrame( EncodedImageCallback::DropReason::kDroppedByMediaOptimizations); + accumulated_update_rect_.Union(video_frame.update_rect()); return; } @@ -977,13 +997,20 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, I420Buffer::Create(cropped_width, cropped_height); // TODO(ilnik): Remove scaling if cropping is too big, as it should never // happen after SinkWants signaled correctly from ReconfigureEncoder. + VideoFrame::UpdateRect update_rect = video_frame.update_rect(); if (crop_width_ < 4 && crop_height_ < 4) { cropped_buffer->CropAndScaleFrom( *video_frame.video_frame_buffer()->ToI420(), crop_width_ / 2, crop_height_ / 2, cropped_width, cropped_height); + update_rect.offset_x -= crop_width_ / 2; + update_rect.offset_y -= crop_height_ / 2; + update_rect.Intersect( + VideoFrame::UpdateRect{0, 0, cropped_width, cropped_height}); + } else { cropped_buffer->ScaleFrom( *video_frame.video_frame_buffer()->ToI420().get()); + update_rect = VideoFrame::UpdateRect{0, 0, cropped_width, cropped_height}; } out_frame = VideoFrame::Builder() .set_video_frame_buffer(cropped_buffer) @@ -991,8 +1018,24 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, .set_timestamp_ms(video_frame.render_time_ms()) .set_rotation(video_frame.rotation()) .set_id(video_frame.id()) + .set_update_rect(update_rect) .build(); out_frame.set_ntp_time_ms(video_frame.ntp_time_ms()); + // Since accumulated_update_rect_ is constructed before cropping, + // we can't trust it. If any changes were pending, we invalidate whole + // frame here. + if (!accumulated_update_rect_.IsEmpty()) { + accumulated_update_rect_ = + VideoFrame::UpdateRect{0, 0, out_frame.width(), out_frame.height()}; + } + } + + if (!accumulated_update_rect_.IsEmpty()) { + accumulated_update_rect_.Union(out_frame.update_rect()); + accumulated_update_rect_.Intersect( + VideoFrame::UpdateRect{0, 0, out_frame.width(), out_frame.height()}); + out_frame.set_update_rect(accumulated_update_rect_); + accumulated_update_rect_.MakeEmptyUpdate(); } TRACE_EVENT_ASYNC_STEP0("webrtc", "Video", video_frame.render_time_ms(), diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index feed55b9bf..861028f104 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -274,6 +274,9 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, absl::optional pending_frame_ RTC_GUARDED_BY(&encoder_queue_); int64_t pending_frame_post_time_us_ RTC_GUARDED_BY(&encoder_queue_); + VideoFrame::UpdateRect accumulated_update_rect_ + RTC_GUARDED_BY(&encoder_queue_); + VideoBitrateAllocationObserver* bitrate_observer_ RTC_GUARDED_BY(&encoder_queue_); absl::optional last_parameters_update_ms_ diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 3497db842e..b419e60c2d 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -373,6 +373,22 @@ class VideoStreamEncoderTest : public ::testing::Test { return frame; } + VideoFrame CreateFrameWithUpdatedPixel(int64_t ntp_time_ms, + rtc::Event* destruction_event, + int offset_x) const { + VideoFrame frame = + VideoFrame::Builder() + .set_video_frame_buffer(new rtc::RefCountedObject( + destruction_event, codec_width_, codec_height_)) + .set_timestamp_rtp(99) + .set_timestamp_ms(99) + .set_rotation(kVideoRotation_0) + .set_update_rect({offset_x, 0, 1, 1}) + .build(); + frame.set_ntp_time_ms(ntp_time_ms); + return frame; + } + VideoFrame CreateFrame(int64_t ntp_time_ms, int width, int height) const { VideoFrame frame = VideoFrame::Builder() @@ -573,6 +589,11 @@ class VideoStreamEncoderTest : public ::testing::Test { return last_framerate_; } + VideoFrame::UpdateRect GetLastUpdateRect() { + rtc::CritScope lock(&local_crit_sect_); + return last_update_rect_; + } + private: int32_t Encode(const VideoFrame& input_image, const CodecSpecificInfo* codec_specific_info, @@ -590,6 +611,7 @@ class VideoStreamEncoderTest : public ::testing::Test { last_input_height_ = input_image.height(); block_encode = block_next_encode_; block_next_encode_ = false; + last_update_rect_ = input_image.update_rect(); } int32_t result = FakeEncoder::Encode(input_image, codec_specific_info, frame_types); @@ -655,6 +677,8 @@ class VideoStreamEncoderTest : public ::testing::Test { bool force_init_encode_failed_ RTC_GUARDED_BY(local_crit_sect_) = false; double rate_factor_ RTC_GUARDED_BY(local_crit_sect_) = 1.0; uint32_t last_framerate_ RTC_GUARDED_BY(local_crit_sect_) = 0; + VideoFrame::UpdateRect last_update_rect_ + RTC_GUARDED_BY(local_crit_sect_) = {0, 0, 0, 0}; }; class TestSink : public VideoStreamEncoder::EncoderSink { @@ -3361,4 +3385,48 @@ TEST_F(VideoStreamEncoderTest, ConfiguresCorrectFrameRate) { video_stream_encoder_->Stop(); } +TEST_F(VideoStreamEncoderTest, AccumulatesUpdateRectOnDroppedFrames) { + VideoFrame::UpdateRect rect; + video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); + + fake_encoder_.BlockNextEncode(); + video_source_.IncomingCapturedFrame( + CreateFrameWithUpdatedPixel(1, nullptr, 0)); + WaitForEncodedFrame(1); + // On the very first frame full update should be forced. + rect = fake_encoder_.GetLastUpdateRect(); + EXPECT_EQ(rect.offset_x, 0); + EXPECT_EQ(rect.offset_y, 0); + EXPECT_EQ(rect.height, codec_height_); + EXPECT_EQ(rect.width, codec_width_); + // Here, the encoder thread will be blocked in the TestEncoder waiting for a + // call to ContinueEncode. + video_source_.IncomingCapturedFrame( + CreateFrameWithUpdatedPixel(2, nullptr, 1)); + ExpectDroppedFrame(); + video_source_.IncomingCapturedFrame( + CreateFrameWithUpdatedPixel(3, nullptr, 10)); + ExpectDroppedFrame(); + fake_encoder_.ContinueEncode(); + WaitForEncodedFrame(3); + // Updates to pixels 1 and 10 should be accumulated to one 10x1 rect. + rect = fake_encoder_.GetLastUpdateRect(); + EXPECT_EQ(rect.offset_x, 1); + EXPECT_EQ(rect.offset_y, 0); + EXPECT_EQ(rect.width, 10); + EXPECT_EQ(rect.height, 1); + + video_source_.IncomingCapturedFrame( + CreateFrameWithUpdatedPixel(4, nullptr, 0)); + WaitForEncodedFrame(4); + // Previous frame was encoded, so no accumulation should happen. + rect = fake_encoder_.GetLastUpdateRect(); + EXPECT_EQ(rect.offset_x, 0); + EXPECT_EQ(rect.offset_y, 0); + EXPECT_EQ(rect.width, 1); + EXPECT_EQ(rect.height, 1); + + video_stream_encoder_->Stop(); +} + } // namespace webrtc