From 9560d7dc58bc379061b9ab9d8d72b137c077fd10 Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Wed, 30 Oct 2019 11:19:47 +0100 Subject: [PATCH] Make update_rect optional in VideoFrame For the automatic content type detection we need to know if the update rect is trusted or just not available. Currently we only care if it's not empty, so in case of no update rect available, full frame resolution was set as a changed region. This CL makes the update_rect field optional but should be a no-op in the current code, as absence of update_rect is treated as a full update via a new getter method |update_rect_or_full_frame()|. Bug: webrtc:11058 Change-Id: I913545b71ac2fc845861549ac34eb1b630012109 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158673 Commit-Queue: Ilya Nikolaevskiy Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#29654} --- api/video/video_frame.cc | 19 +++++++++---------- api/video/video_frame.h | 21 +++++++++++++++------ media/base/video_broadcaster.cc | 8 ++++---- video/video_stream_encoder.cc | 14 +++++++++++++- video/video_stream_encoder.h | 1 + 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/api/video/video_frame.cc b/api/video/video_frame.cc index 2ef8d8d196..0e6a611dd8 100644 --- a/api/video/video_frame.cc +++ b/api/video/video_frame.cc @@ -142,8 +142,7 @@ VideoFrame::VideoFrame(const rtc::scoped_refptr& buffer, timestamp_rtp_(0), ntp_time_ms_(0), timestamp_us_(timestamp_us), - rotation_(rotation), - update_rect_{0, 0, buffer->width(), buffer->height()} {} + rotation_(rotation) {} VideoFrame::VideoFrame(const rtc::scoped_refptr& buffer, uint32_t timestamp_rtp, @@ -153,8 +152,7 @@ VideoFrame::VideoFrame(const rtc::scoped_refptr& buffer, timestamp_rtp_(timestamp_rtp), ntp_time_ms_(0), timestamp_us_(render_time_ms * rtc::kNumMicrosecsPerMillisec), - rotation_(rotation), - update_rect_{0, 0, buffer->width(), buffer->height()} { + rotation_(rotation) { RTC_DCHECK(buffer); } @@ -174,13 +172,14 @@ VideoFrame::VideoFrame(uint16_t id, timestamp_us_(timestamp_us), rotation_(rotation), color_space_(color_space), - update_rect_(update_rect.value_or(UpdateRect{ - 0, 0, video_frame_buffer_->width(), video_frame_buffer_->height()})), + update_rect_(update_rect), packet_infos_(std::move(packet_infos)) { - RTC_DCHECK_GE(update_rect_.offset_x, 0); - RTC_DCHECK_GE(update_rect_.offset_y, 0); - RTC_DCHECK_LE(update_rect_.offset_x + update_rect_.width, width()); - RTC_DCHECK_LE(update_rect_.offset_y + update_rect_.height, height()); + if (update_rect_) { + RTC_DCHECK_GE(update_rect_->offset_x, 0); + RTC_DCHECK_GE(update_rect_->offset_y, 0); + RTC_DCHECK_LE(update_rect_->offset_x + update_rect_->width, width()); + RTC_DCHECK_LE(update_rect_->offset_y + update_rect_->height, height()); + } } VideoFrame::~VideoFrame() = default; diff --git a/api/video/video_frame.h b/api/video/video_frame.h index 127e62fed9..51cee649f1 100644 --- a/api/video/video_frame.h +++ b/api/video/video_frame.h @@ -172,9 +172,14 @@ class RTC_EXPORT VideoFrame { return video_frame_buffer()->type() == VideoFrameBuffer::Type::kNative; } - // Always initialized to whole frame update, can be set by Builder or manually - // by |set_update_rect|. - UpdateRect update_rect() const { return update_rect_; } + bool has_update_rect() const { return update_rect_.has_value(); } + + // Returns update_rect set by the builder or set_update_rect() or whole frame + // rect if no update rect is available. + UpdateRect update_rect() const { + return update_rect_.value_or(UpdateRect{0, 0, width(), height()}); + } + // Rectangle must be within the frame dimensions. void set_update_rect(const VideoFrame::UpdateRect& update_rect) { RTC_DCHECK_GE(update_rect.offset_x, 0); @@ -184,6 +189,8 @@ class RTC_EXPORT VideoFrame { update_rect_ = update_rect; } + void clear_update_rect() { update_rect_ = absl::nullopt; } + // Get information about packets used to assemble this video frame. Might be // empty if the information isn't available. const RtpPacketInfos& packet_infos() const { return packet_infos_; } @@ -210,9 +217,11 @@ class RTC_EXPORT VideoFrame { int64_t timestamp_us_; VideoRotation rotation_; absl::optional color_space_; - // Updated since the last frame area. Unless set explicitly, will always be - // a full frame rectangle. - UpdateRect update_rect_; + // Updated since the last frame area. If present it means that the bounding + // box of all the changes is within the rectangular area and is close to it. + // If absent, it means that there's no information about the change at all and + // update_rect() will return a rectangle corresponding to the entire frame. + absl::optional update_rect_; // Information about packets used to assemble this video frame. This is needed // by |SourceTracker| when the frame is delivered to the RTCRtpReceiver's // MediaStreamTrack, in order to implement getContributingSources(). See: diff --git a/media/base/video_broadcaster.cc b/media/base/video_broadcaster.cc index ab7e622ea7..436bd5348e 100644 --- a/media/base/video_broadcaster.cc +++ b/media/base/video_broadcaster.cc @@ -79,11 +79,11 @@ 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. + } else if (!previous_frame_sent_to_all_sinks_ && frame.has_update_rect()) { + // Since last frame was not sent to some sinks, no reliable update + // information is available, so we need to clear the update rect. webrtc::VideoFrame copy = frame; - copy.set_update_rect( - webrtc::VideoFrame::UpdateRect{0, 0, frame.width(), frame.height()}); + copy.clear_update_rect(); sink_pair.sink->OnFrame(copy); } else { sink_pair.sink->OnFrame(frame); diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 549a280407..a986baf6c8 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -516,6 +516,7 @@ VideoStreamEncoder::VideoStreamEncoder( dropped_frame_count_(0), pending_frame_post_time_us_(0), accumulated_update_rect_{0, 0, 0, 0}, + accumulated_update_rect_is_valid_(true), bitrate_observer_(nullptr), fec_controller_override_(nullptr), force_disable_frame_dropper_(false), @@ -1085,6 +1086,7 @@ void VideoStreamEncoder::OnFrame(const VideoFrame& video_frame) { encoder_queue_.PostTask([this, incoming_frame]() { RTC_DCHECK_RUN_ON(&encoder_queue_); accumulated_update_rect_.Union(incoming_frame.update_rect()); + accumulated_update_rect_is_valid_ &= incoming_frame.has_update_rect(); }); return; } @@ -1119,6 +1121,7 @@ void VideoStreamEncoder::OnFrame(const VideoFrame& video_frame) { encoder_stats_observer_->OnFrameDropped( VideoStreamEncoderObserver::DropReason::kEncoderQueue); accumulated_update_rect_.Union(incoming_frame.update_rect()); + accumulated_update_rect_is_valid_ &= incoming_frame.has_update_rect(); } if (log_stats) { RTC_LOG(LS_INFO) << "Number of frames: captured " @@ -1324,6 +1327,7 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, encoder_stats_observer_->OnFrameDropped( VideoStreamEncoderObserver::DropReason::kEncoderQueue); accumulated_update_rect_.Union(pending_frame_->update_rect()); + accumulated_update_rect_is_valid_ &= pending_frame_->has_update_rect(); } if (DropDueToSize(video_frame.size())) { @@ -1349,6 +1353,7 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, // Ensure that any previously stored frame is dropped. pending_frame_.reset(); accumulated_update_rect_.Union(video_frame.update_rect()); + accumulated_update_rect_is_valid_ &= video_frame.has_update_rect(); } return; } @@ -1367,6 +1372,7 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, pending_frame_.reset(); TraceFrameDropStart(); accumulated_update_rect_.Union(video_frame.update_rect()); + accumulated_update_rect_is_valid_ &= video_frame.has_update_rect(); } return; } @@ -1391,6 +1397,7 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, OnDroppedFrame( EncodedImageCallback::DropReason::kDroppedByMediaOptimizations); accumulated_update_rect_.Union(video_frame.update_rect()); + accumulated_update_rect_is_valid_ &= video_frame.has_update_rect(); return; } @@ -1508,16 +1515,21 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, if (!accumulated_update_rect_.IsEmpty()) { accumulated_update_rect_ = VideoFrame::UpdateRect{0, 0, out_frame.width(), out_frame.height()}; + accumulated_update_rect_is_valid_ = false; } } - if (!accumulated_update_rect_.IsEmpty()) { + if (!accumulated_update_rect_is_valid_) { + out_frame.clear_update_rect(); + } else if (!accumulated_update_rect_.IsEmpty() && + out_frame.has_update_rect()) { 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(); } + accumulated_update_rect_is_valid_ = true; TRACE_EVENT_ASYNC_STEP0("webrtc", "Video", video_frame.render_time_ms(), "Encode"); diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 3b096329fe..1b76e2bd44 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -336,6 +336,7 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, VideoFrame::UpdateRect accumulated_update_rect_ RTC_GUARDED_BY(&encoder_queue_); + bool accumulated_update_rect_is_valid_ RTC_GUARDED_BY(&encoder_queue_); VideoBitrateAllocationObserver* bitrate_observer_ RTC_GUARDED_BY(&encoder_queue_);