VideoStreamEncoder: Clean up drop handling and update rects.

The change adds dropped frame reporting for previously dropped frame
and also cleans up the colon list of the VSE.

Bug: None
Change-Id: Iad1c084739e5392ded4f100d940b45adf9b561ef
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/327800
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41225}

api/video/video_frame.h

@@ -33,10 +33,10 @@ class RTC_EXPORT VideoFrame {
   static constexpr uint16_t kNotSetId = 0;
 
   struct RTC_EXPORT UpdateRect {
-    int offset_x;
+    int offset_x = 0;
-    int offset_y;
+    int offset_y = 0;
-    int width;
+    int width = 0;
-    int height;
+    int height = 0;
 
     // Makes this UpdateRect a bounding box of this and other rect.
     void Union(const UpdateRect& other);

call/video_send_stream.h

@@ -113,6 +113,7 @@ class VideoSendStream {
     uint64_t total_encoded_bytes_target = 0;
     uint32_t frames = 0;
     uint32_t frames_dropped_by_capturer = 0;
+    uint32_t frames_dropped_by_bad_timestamp = 0;
     uint32_t frames_dropped_by_encoder_queue = 0;
     uint32_t frames_dropped_by_rate_limiter = 0;
     uint32_t frames_dropped_by_congestion_window = 0;

video/send_statistics_proxy.cc

@@ -658,6 +658,10 @@ void SendStatisticsProxy::UmaSamplesContainer::UpdateHistograms(
              << current_stats.frames_dropped_by_capturer << "\n";
   RTC_HISTOGRAMS_COUNTS_1000(kIndex, uma_prefix_ + "DroppedFrames.Capturer",
                              current_stats.frames_dropped_by_capturer);
+  log_stream << uma_prefix_ << "DroppedFrames.BadTimestamp"
+             << current_stats.frames_dropped_by_bad_timestamp << "\n";
+  RTC_HISTOGRAMS_COUNTS_1000(kIndex, uma_prefix_ + "DroppedFrames.BadTimestamp",
+                             current_stats.frames_dropped_by_bad_timestamp);
   log_stream << uma_prefix_ << "DroppedFrames.EncoderQueue "
              << current_stats.frames_dropped_by_encoder_queue << "\n";
   RTC_HISTOGRAMS_COUNTS_1000(kIndex, uma_prefix_ + "DroppedFrames.EncoderQueue",
@@ -1097,6 +1101,9 @@ void SendStatisticsProxy::OnFrameDropped(DropReason reason) {
     case DropReason::kSource:
       ++stats_.frames_dropped_by_capturer;
       break;
+    case DropReason::kBadTimestamp:
+      ++stats_.frames_dropped_by_bad_timestamp;
+      break;
     case DropReason::kEncoderQueue:
       ++stats_.frames_dropped_by_encoder_queue;
       break;

video/video_stream_encoder.cc

@@ -94,6 +94,18 @@ int GetNumSpatialLayers(const VideoCodec& codec) {
   }
 }
 
+absl::optional<EncodedImageCallback::DropReason> MaybeConvertDropReason(
+    VideoStreamEncoderObserver::DropReason reason) {
+  switch (reason) {
+    case VideoStreamEncoderObserver::DropReason::kMediaOptimization:
+      return EncodedImageCallback::DropReason::kDroppedByMediaOptimizations;
+    case VideoStreamEncoderObserver::DropReason::kEncoder:
+      return EncodedImageCallback::DropReason::kDroppedByEncoder;
+    default:
+      return absl::nullopt;
+  }
+}
+
 bool RequiresEncoderReset(const VideoCodec& prev_send_codec,
                           const VideoCodec& new_send_codec,
                           bool was_encode_called_since_last_initialization) {
@@ -647,7 +659,6 @@ VideoStreamEncoder::VideoStreamEncoder(
     : field_trials_(field_trials),
       worker_queue_(TaskQueueBase::Current()),
       number_of_cores_(number_of_cores),
-      sink_(nullptr),
       settings_(settings),
       allocation_cb_type_(allocation_cb_type),
       rate_control_settings_(
@@ -661,39 +672,12 @@ VideoStreamEncoder::VideoStreamEncoder(
                             ? encoder_selector_from_constructor_
                             : encoder_selector_from_factory_.get()),
       encoder_stats_observer_(encoder_stats_observer),
-      cadence_callback_(*this),
       frame_cadence_adapter_(std::move(frame_cadence_adapter)),
-      encoder_initialized_(false),
-      max_framerate_(-1),
-      pending_encoder_reconfiguration_(false),
-      pending_encoder_creation_(false),
-      crop_width_(0),
-      crop_height_(0),
-      encoder_target_bitrate_bps_(absl::nullopt),
-      max_data_payload_length_(0),
-      encoder_paused_and_dropped_frame_(false),
-      was_encode_called_since_last_initialization_(false),
-      encoder_failed_(false),
       clock_(clock),
-      last_captured_timestamp_(0),
       delta_ntp_internal_ms_(clock_->CurrentNtpInMilliseconds() -
                              clock_->TimeInMilliseconds()),
       last_frame_log_ms_(clock_->TimeInMilliseconds()),
-      captured_frame_count_(0),
-      dropped_frame_cwnd_pushback_count_(0),
-      dropped_frame_encoder_block_count_(0),
-      pending_frame_post_time_us_(0),
-      accumulated_update_rect_{0, 0, 0, 0},
-      accumulated_update_rect_is_valid_(true),
-      animation_start_time_(Timestamp::PlusInfinity()),
-      cap_resolution_due_to_video_content_(false),
-      expect_resize_state_(ExpectResizeState::kNoResize),
-      fec_controller_override_(nullptr),
-      force_disable_frame_dropper_(false),
-      pending_frame_drops_(0),
-      cwnd_frame_counter_(0),
       next_frame_types_(1, VideoFrameType::kVideoFrameDelta),
-      frame_encode_metadata_writer_(this),
       automatic_animation_detection_experiment_(
           ParseAutomatincAnimationDetectionFieldTrial()),
       input_state_provider_(encoder_stats_observer),
@@ -704,7 +688,6 @@ VideoStreamEncoder::VideoStreamEncoder(
       degradation_preference_manager_(
           std::make_unique<DegradationPreferenceManager>(
               video_stream_adapter_.get())),
-      adaptation_constraints_(),
       stream_resource_manager_(&input_state_provider_,
                                encoder_stats_observer,
                                clock_,
@@ -1538,8 +1521,8 @@ void VideoStreamEncoder::OnFrame(Timestamp post_time,
                         << incoming_frame.ntp_time_ms()
                         << " <= " << last_captured_timestamp_
                         << ") for incoming frame. Dropping.";
-    accumulated_update_rect_.Union(incoming_frame.update_rect());
+    ProcessDroppedFrame(incoming_frame,
-    accumulated_update_rect_is_valid_ &= incoming_frame.has_update_rect();
+                        VideoStreamEncoderObserver::DropReason::kBadTimestamp);
     return;
   }
 
@@ -1565,18 +1548,17 @@ void VideoStreamEncoder::OnFrame(Timestamp post_time,
       // Frame drop by congestion window pushback. Do not encode this
       // frame.
       ++dropped_frame_cwnd_pushback_count_;
-      encoder_stats_observer_->OnFrameDropped(
-          VideoStreamEncoderObserver::DropReason::kCongestionWindow);
     } else {
       // There is a newer frame in flight. Do not encode this frame.
       RTC_LOG(LS_VERBOSE)
           << "Incoming frame dropped due to that the encoder is blocked.";
       ++dropped_frame_encoder_block_count_;
-      encoder_stats_observer_->OnFrameDropped(
-          VideoStreamEncoderObserver::DropReason::kEncoderQueue);
     }
-    accumulated_update_rect_.Union(incoming_frame.update_rect());
+    ProcessDroppedFrame(
-    accumulated_update_rect_is_valid_ &= incoming_frame.has_update_rect();
+        incoming_frame,
+        cwnd_frame_drop
+            ? VideoStreamEncoderObserver::DropReason::kCongestionWindow
+            : VideoStreamEncoderObserver::DropReason::kEncoderQueue);
   }
   if (log_stats) {
     RTC_LOG(LS_INFO) << "Number of frames: captured " << captured_frame_count_
@@ -1813,10 +1795,8 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame,
   // Because pending frame will be dropped in any case, we need to
   // remember its updated region.
   if (pending_frame_) {
-    encoder_stats_observer_->OnFrameDropped(
+    ProcessDroppedFrame(*pending_frame_,
-        VideoStreamEncoderObserver::DropReason::kEncoderQueue);
+                        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())) {
@@ -1830,10 +1810,8 @@ 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());
+      ProcessDroppedFrame(
-      accumulated_update_rect_is_valid_ &= video_frame.has_update_rect();
+          video_frame, VideoStreamEncoderObserver::DropReason::kEncoderQueue);
-      encoder_stats_observer_->OnFrameDropped(
-          VideoStreamEncoderObserver::DropReason::kEncoderQueue);
     }
     return;
   }
@@ -1851,10 +1829,8 @@ 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());
+      ProcessDroppedFrame(
-      accumulated_update_rect_is_valid_ &= video_frame.has_update_rect();
+          video_frame, VideoStreamEncoderObserver::DropReason::kEncoderQueue);
-      encoder_stats_observer_->OnFrameDropped(
-          VideoStreamEncoderObserver::DropReason::kEncoderQueue);
     }
     return;
   }
@@ -1876,10 +1852,9 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame,
                 ? last_encoder_rate_settings_->encoder_target.bps()
                 : 0)
         << ", input frame rate " << framerate_fps;
-    OnDroppedFrame(
+    ProcessDroppedFrame(
-        EncodedImageCallback::DropReason::kDroppedByMediaOptimizations);
+        video_frame,
-    accumulated_update_rect_.Union(video_frame.update_rect());
+        VideoStreamEncoderObserver::DropReason::kMediaOptimization);
-    accumulated_update_rect_is_valid_ &= video_frame.has_update_rect();
     return;
   }
 
@@ -2225,16 +2200,6 @@ EncodedImageCallback::Result VideoStreamEncoder::OnEncodedImage(
 }
 
 void VideoStreamEncoder::OnDroppedFrame(DropReason reason) {
-  switch (reason) {
-    case DropReason::kDroppedByMediaOptimizations:
-      encoder_stats_observer_->OnFrameDropped(
-          VideoStreamEncoderObserver::DropReason::kMediaOptimization);
-      break;
-    case DropReason::kDroppedByEncoder:
-      encoder_stats_observer_->OnFrameDropped(
-          VideoStreamEncoderObserver::DropReason::kEncoder);
-      break;
-  }
   sink_->OnDroppedFrame(reason);
   encoder_queue_.PostTask([this, reason] {
     RTC_DCHECK_RUN_ON(&encoder_queue_);
@@ -2617,4 +2582,16 @@ void VideoStreamEncoder::RemoveRestrictionsListenerForTesting(
   event.Wait(rtc::Event::kForever);
 }
 
+// RTC_RUN_ON(&encoder_queue_)
+void VideoStreamEncoder::ProcessDroppedFrame(
+    const VideoFrame& frame,
+    VideoStreamEncoderObserver::DropReason reason) {
+  accumulated_update_rect_.Union(frame.update_rect());
+  accumulated_update_rect_is_valid_ &= frame.has_update_rect();
+  if (auto converted_reason = MaybeConvertDropReason(reason)) {
+    OnDroppedFrame(*converted_reason);
+  }
+  encoder_stats_observer_->OnFrameDropped(reason);
+}
+
 }  // namespace webrtc

video/video_stream_encoder.h

@@ -266,12 +266,16 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
       const EncodedImage& encoded_image,
       const CodecSpecificInfo* codec_specific_info);
 
+  void ProcessDroppedFrame(const VideoFrame& frame,
+                           VideoStreamEncoderObserver::DropReason reason)
+      RTC_RUN_ON(&encoder_queue_);
+
   const FieldTrialsView& field_trials_;
   TaskQueueBase* const worker_queue_;
 
   const int number_of_cores_;
 
-  EncoderSink* sink_;
+  EncoderSink* sink_ = nullptr;
   const VideoStreamEncoderSettings settings_;
   const BitrateAllocationCallbackType allocation_cb_type_;
   const RateControlSettings rate_control_settings_;
@@ -287,7 +291,7 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
   VideoStreamEncoderObserver* const encoder_stats_observer_;
   // Adapter that avoids public inheritance of the cadence adapter's callback
   // interface.
-  CadenceCallback cadence_callback_;
+  CadenceCallback cadence_callback_{*this};
   // Frame cadence encoder adapter. Frames enter this adapter first, and it then
   // forwards them to our OnFrame method.
   std::unique_ptr<FrameCadenceAdapterInterface> frame_cadence_adapter_
@@ -296,70 +300,74 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
   VideoEncoderConfig encoder_config_ RTC_GUARDED_BY(&encoder_queue_);
   std::unique_ptr<VideoEncoder> encoder_ RTC_GUARDED_BY(&encoder_queue_)
       RTC_PT_GUARDED_BY(&encoder_queue_);
-  bool encoder_initialized_;
+  bool encoder_initialized_ = false;
   std::unique_ptr<VideoBitrateAllocator> rate_allocator_
       RTC_GUARDED_BY(&encoder_queue_) RTC_PT_GUARDED_BY(&encoder_queue_);
-  int max_framerate_ RTC_GUARDED_BY(&encoder_queue_);
+  int max_framerate_ RTC_GUARDED_BY(&encoder_queue_) = -1;
 
   // Set when ConfigureEncoder has been called in order to lazy reconfigure the
   // encoder on the next frame.
-  bool pending_encoder_reconfiguration_ RTC_GUARDED_BY(&encoder_queue_);
+  bool pending_encoder_reconfiguration_ RTC_GUARDED_BY(&encoder_queue_) = false;
   // Set when configuration must create a new encoder object, e.g.,
   // because of a codec change.
-  bool pending_encoder_creation_ RTC_GUARDED_BY(&encoder_queue_);
+  bool pending_encoder_creation_ RTC_GUARDED_BY(&encoder_queue_) = false;
   absl::InlinedVector<SetParametersCallback, 2> encoder_configuration_callbacks_
       RTC_GUARDED_BY(&encoder_queue_);
 
   absl::optional<VideoFrameInfo> last_frame_info_
       RTC_GUARDED_BY(&encoder_queue_);
-  int crop_width_ RTC_GUARDED_BY(&encoder_queue_);
+  int crop_width_ RTC_GUARDED_BY(&encoder_queue_) = 0;
-  int crop_height_ RTC_GUARDED_BY(&encoder_queue_);
+  int crop_height_ RTC_GUARDED_BY(&encoder_queue_) = 0;
   absl::optional<uint32_t> encoder_target_bitrate_bps_
       RTC_GUARDED_BY(&encoder_queue_);
-  size_t max_data_payload_length_ RTC_GUARDED_BY(&encoder_queue_);
+  size_t max_data_payload_length_ RTC_GUARDED_BY(&encoder_queue_) = 0;
   absl::optional<EncoderRateSettings> last_encoder_rate_settings_
       RTC_GUARDED_BY(&encoder_queue_);
-  bool encoder_paused_and_dropped_frame_ RTC_GUARDED_BY(&encoder_queue_);
+  bool encoder_paused_and_dropped_frame_ RTC_GUARDED_BY(&encoder_queue_) =
+      false;
 
   // Set to true if at least one frame was sent to encoder since last encoder
   // initialization.
   bool was_encode_called_since_last_initialization_
-      RTC_GUARDED_BY(&encoder_queue_);
+      RTC_GUARDED_BY(&encoder_queue_) = false;
 
-  bool encoder_failed_ RTC_GUARDED_BY(&encoder_queue_);
+  bool encoder_failed_ RTC_GUARDED_BY(&encoder_queue_) = false;
   Clock* const clock_;
 
   // Used to make sure incoming time stamp is increasing for every frame.
-  int64_t last_captured_timestamp_ RTC_GUARDED_BY(&encoder_queue_);
+  int64_t last_captured_timestamp_ RTC_GUARDED_BY(&encoder_queue_) = 0;
   // Delta used for translating between NTP and internal timestamps.
   const int64_t delta_ntp_internal_ms_ RTC_GUARDED_BY(&encoder_queue_);
 
   int64_t last_frame_log_ms_ RTC_GUARDED_BY(&encoder_queue_);
-  int captured_frame_count_ RTC_GUARDED_BY(&encoder_queue_);
+  int captured_frame_count_ RTC_GUARDED_BY(&encoder_queue_) = 0;
-  int dropped_frame_cwnd_pushback_count_ RTC_GUARDED_BY(&encoder_queue_);
+  int dropped_frame_cwnd_pushback_count_ RTC_GUARDED_BY(&encoder_queue_) = 0;
-  int dropped_frame_encoder_block_count_ RTC_GUARDED_BY(&encoder_queue_);
+  int dropped_frame_encoder_block_count_ RTC_GUARDED_BY(&encoder_queue_) = 0;
   absl::optional<VideoFrame> pending_frame_ RTC_GUARDED_BY(&encoder_queue_);
-  int64_t pending_frame_post_time_us_ RTC_GUARDED_BY(&encoder_queue_);
+  int64_t pending_frame_post_time_us_ RTC_GUARDED_BY(&encoder_queue_) = 0;
 
   VideoFrame::UpdateRect accumulated_update_rect_
       RTC_GUARDED_BY(&encoder_queue_);
-  bool accumulated_update_rect_is_valid_ RTC_GUARDED_BY(&encoder_queue_);
+  bool accumulated_update_rect_is_valid_ RTC_GUARDED_BY(&encoder_queue_) = true;
 
   // Used for automatic content type detection.
   absl::optional<VideoFrame::UpdateRect> last_update_rect_
       RTC_GUARDED_BY(&encoder_queue_);
-  Timestamp animation_start_time_ RTC_GUARDED_BY(&encoder_queue_);
+  Timestamp animation_start_time_ RTC_GUARDED_BY(&encoder_queue_) =
-  bool cap_resolution_due_to_video_content_ RTC_GUARDED_BY(&encoder_queue_);
+      Timestamp::PlusInfinity();
+  bool cap_resolution_due_to_video_content_ RTC_GUARDED_BY(&encoder_queue_) =
+      false;
   // Used to correctly ignore changes in update_rect introduced by
   // resize triggered by animation detection.
   enum class ExpectResizeState {
     kNoResize,              // Normal operation.
     kResize,                // Resize was triggered by the animation detection.
     kFirstFrameAfterResize  // Resize observed.
-  } expect_resize_state_ RTC_GUARDED_BY(&encoder_queue_);
+  } expect_resize_state_ RTC_GUARDED_BY(&encoder_queue_) =
+      ExpectResizeState::kNoResize;
 
   FecControllerOverride* fec_controller_override_
-      RTC_GUARDED_BY(&encoder_queue_);
+      RTC_GUARDED_BY(&encoder_queue_) = nullptr;
   absl::optional<int64_t> last_parameters_update_ms_
       RTC_GUARDED_BY(&encoder_queue_);
   absl::optional<int64_t> last_encode_info_ms_ RTC_GUARDED_BY(&encoder_queue_);
@@ -372,18 +380,18 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
   // disabled if VideoEncoder::GetEncoderInfo() indicates that the encoder has a
   // trusted rate controller. This is determined on a per-frame basis, as the
   // encoder behavior might dynamically change.
-  bool force_disable_frame_dropper_ RTC_GUARDED_BY(&encoder_queue_);
+  bool force_disable_frame_dropper_ RTC_GUARDED_BY(&encoder_queue_) = false;
   // Incremented on worker thread whenever `frame_dropper_` determines that a
   // frame should be dropped. Decremented on whichever thread runs
   // OnEncodedImage(), which is only called by one thread but not necessarily
   // the worker thread.
-  std::atomic<int> pending_frame_drops_;
+  std::atomic<int> pending_frame_drops_{0};
 
   // Congestion window frame drop ratio (drop 1 in every
   // cwnd_frame_drop_interval_ frames).
   absl::optional<int> cwnd_frame_drop_interval_ RTC_GUARDED_BY(&encoder_queue_);
   // Frame counter for congestion window frame drop.
-  int cwnd_frame_counter_ RTC_GUARDED_BY(&encoder_queue_);
+  int cwnd_frame_counter_ RTC_GUARDED_BY(&encoder_queue_) = 0;
 
   std::unique_ptr<EncoderBitrateAdjuster> bitrate_adjuster_
       RTC_GUARDED_BY(&encoder_queue_);
@@ -392,7 +400,7 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
   // turn this into a simple bool `pending_keyframe_request_`.
   std::vector<VideoFrameType> next_frame_types_ RTC_GUARDED_BY(&encoder_queue_);
 
-  FrameEncodeMetadataWriter frame_encode_metadata_writer_;
+  FrameEncodeMetadataWriter frame_encode_metadata_writer_{this};
 
   struct AutomaticAnimationDetectionExperiment {
     bool enabled = false;

video/video_stream_encoder_observer.h

@@ -58,6 +58,7 @@ class VideoStreamEncoderObserver : public CpuOveruseMetricsObserver {
 
   enum class DropReason {
     kSource,
+    kBadTimestamp,
     kEncoderQueue,
     kEncoder,
     kMediaOptimization,