Propagate VideoFrame::UpdateRect to encoder
Accumulate it in all places where frames can be dropped before they reach
the encoder.
Reset UpdateRect in VideoBroadcaster if frame the previous frame is dropped.
No accumulation is done here since it's supposed to be a brief occusion then
configuration have changed.
Bug: webrtc:10310
Change-Id: I2813ecd009eb730bd99ffa0a02f979091b56bf80
Reviewed-on: https://webrtc-review.googlesource.com/c/123102
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26711}diff --git a/api/video/video_frame.cc b/api/video/video_frame.cc
index e91eede..7a71f43 100644
--- a/api/video/video_frame.cc
+++ b/api/video/video_frame.cc
@@ -10,11 +10,55 @@
#include "api/video/video_frame.h"
+#include <algorithm>
+
#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 126c3d0..c7dd02a 100644
--- a/api/video/video_frame.h
+++ b/api/video/video_frame.h
@@ -31,6 +31,17 @@
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 a2009f2..06001ac 100644
--- a/media/base/video_broadcaster.cc
+++ b/media/base/video_broadcaster.cc
@@ -28,6 +28,10 @@
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 @@
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 @@
// 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 @@
.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 20111bc..898ef2a 100644
--- a/media/base/video_broadcaster.h
+++ b/media/base/video_broadcaster.h
@@ -60,6 +60,8 @@
VideoSinkWants current_wants_ RTC_GUARDED_BY(sinks_and_wants_lock_);
rtc::scoped_refptr<webrtc::VideoFrameBuffer> 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 3d184bd..d547440 100644
--- a/media/engine/simulcast_encoder_adapter.cc
+++ b/media/engine/simulcast_encoder_adapter.cc
@@ -418,6 +418,8 @@
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 5b3a657..4493dd7 100644
--- a/modules/video_coding/video_sender.cc
+++ b/modules/video_coding/video_sender.cc
@@ -199,6 +199,10 @@
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 fb9dd8b..8436dd8 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -388,6 +388,7 @@
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 @@
<< 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 @@
++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 @@
<< 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 @@
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 @@
} 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 @@
// 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 @@
<< ", input frame rate " << framerate_fps;
OnDroppedFrame(
EncodedImageCallback::DropReason::kDroppedByMediaOptimizations);
+ accumulated_update_rect_.Union(video_frame.update_rect());
return;
}
@@ -977,13 +997,20 @@
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 @@
.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 feed55b..861028f 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -274,6 +274,9 @@
absl::optional<VideoFrame> 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<int64_t> last_parameters_update_ms_
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index 3497db8..b419e60 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -373,6 +373,22 @@
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<TestBuffer>(
+ 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 @@
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 @@
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 @@
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 @@
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
