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}

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;

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.

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() {

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<webrtc::VideoFrameBuffer> black_frame_buffer_;
+  bool previous_frame_sent_to_all_sinks_ RTC_GUARDED_BY(sinks_and_wants_lock_) =
+      true;
 };
 
 }  // namespace rtc

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())

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())

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(),

video/video_stream_encoder.h

@@ -274,6 +274,9 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
   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_

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<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 @@ 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