diff --git a/api/video/video_frame.cc b/api/video/video_frame.cc index 7a71f43493..e91eedede8 100644 --- a/api/video/video_frame.cc +++ b/api/video/video_frame.cc @@ -10,55 +10,11 @@ #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 c7dd02a7b1..126c3d05de 100644 --- a/api/video/video_frame.h +++ b/api/video/video_frame.h @@ -31,17 +31,6 @@ 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 06001acc06..a2009f21b3 100644 --- a/media/base/video_broadcaster.cc +++ b/media/base/video_broadcaster.cc @@ -28,10 +28,6 @@ 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(); } @@ -56,7 +52,6 @@ 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) { @@ -65,8 +60,6 @@ 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) { @@ -79,23 +72,10 @@ 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 898ef2ac9a..20111bcac6 100644 --- a/media/base/video_broadcaster.h +++ b/media/base/video_broadcaster.h @@ -60,8 +60,6 @@ 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 d5474405cf..3d184bdf01 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -418,8 +418,6 @@ 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 4493dd7c67..5b3a657878 100644 --- a/modules/video_coding/video_sender.cc +++ b/modules/video_coding/video_sender.cc @@ -199,10 +199,6 @@ 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 8436dd8a3d..fb9dd8b8f1 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -388,7 +388,6 @@ 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), @@ -759,10 +758,6 @@ 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; } @@ -795,7 +790,6 @@ 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 " @@ -884,9 +878,6 @@ 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, @@ -914,14 +905,6 @@ 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); @@ -938,7 +921,6 @@ 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; } @@ -956,7 +938,6 @@ 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; } @@ -976,7 +957,6 @@ 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; } @@ -997,20 +977,13 @@ 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) @@ -1018,24 +991,8 @@ 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 861028f104..feed55b9bf 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -274,9 +274,6 @@ 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 b419e60c2d..3497db842e 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -373,22 +373,6 @@ 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() @@ -589,11 +573,6 @@ 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, @@ -611,7 +590,6 @@ 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); @@ -677,8 +655,6 @@ 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 { @@ -3385,48 +3361,4 @@ 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