Deprecate EncodedImageCallback::OnDroppedFrame. All notifications by a video encoder about a dropped frame should be done via EncodedImageCallback::OnFrameDropped(uint32_t rtp_timestamp, int spatial_id, bool is_end_of_temporal_unit) instead. This allows WebRTC to reason about which frame it is an what the consequences are. Note that in a future CL, the old method will be removed and the new one will be made pure virtual. It will be made mandatory that encoders report the end_of_temporal_unit in either EncodedImage and or via the OnFrameDropped callback, for each frame. Bug: webrtc:467444018 Change-Id: Ifb03cc06184a4c7106ffd2d8c33c1f9892b5dd12 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/451940 Reviewed-by: Sergey Silkin <ssilkin@webrtc.org> Commit-Queue: Erik Språng <sprang@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47419}
diff --git a/api/test/mock_video_encoder.h b/api/test/mock_video_encoder.h index d166642..8f02b4e 100644 --- a/api/test/mock_video_encoder.h +++ b/api/test/mock_video_encoder.h
@@ -31,7 +31,6 @@ OnEncodedImage, (const EncodedImage&, const CodecSpecificInfo*), (override)); - MOCK_METHOD(void, OnDroppedFrame, (DropReason reason), (override)); MOCK_METHOD(void, OnFrameDropped, (uint32_t rtp_timestamp,
diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h index 1110b44..8ff88bb 100644 --- a/api/video_codecs/video_encoder.h +++ b/api/video_codecs/video_encoder.h
@@ -68,8 +68,7 @@ // kDroppedByMediaOptimizations - dropped by MediaOptimizations (for rate // limiting purposes). // kDroppedByEncoder - dropped by encoder's internal rate limiter. - // TODO: bugs.webrtc.org/467444018 - Deprecate and remove after a transition - // period. + // TODO: bugs.webrtc.org/467444018 - Remove when OnDroppedFrame is deleted. enum class DropReason : uint8_t { kDroppedByMediaOptimizations, kDroppedByEncoder @@ -80,8 +79,7 @@ const EncodedImage& encoded_image, const CodecSpecificInfo* codec_specific_info) = 0; - // TODO: bugs.webrtc.org/467444018 - Deprecate and remove after a transition - // period. + [[deprecated("bugs.webrtc.org/467444018 -Use OnFrameDropped instead.")]] virtual void OnDroppedFrame(DropReason /* reason */) {} // Callback function called when an encoder has decided to drop a frame.
diff --git a/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc b/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc index 01138cf..4b82660 100644 --- a/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc +++ b/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc
@@ -336,14 +336,6 @@ } } -void QualityAnalyzingVideoEncoder::OnDroppedFrame( - EncodedImageCallback::DropReason reason) { - MutexLock lock(&mutex_); - analyzer_->OnFrameDropped(peer_name_); - RTC_DCHECK(delegate_callback_); - delegate_callback_->OnDroppedFrame(reason); -} - void QualityAnalyzingVideoEncoder::OnFrameDropped( uint32_t rtp_timestamp, int spatial_id,
diff --git a/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.h b/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.h index bfa7cc9..0094d22 100644 --- a/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.h +++ b/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.h
@@ -88,8 +88,6 @@ EncodedImageCallback::Result OnEncodedImage( const EncodedImage& encoded_image, const CodecSpecificInfo* codec_specific_info) override; - // TODO: webrtc:467444018 - Remove after usage shifts to OnFrameDropped. - void OnDroppedFrame(DropReason reason) override; void OnFrameDropped(uint32_t rtp_timestamp, int spatial_id, bool is_end_of_temporal_unit) override;
diff --git a/video/corruption_detection/evaluation/webrtc_picture_pair_provider.cc b/video/corruption_detection/evaluation/webrtc_picture_pair_provider.cc index 509157a..324bf8c 100644 --- a/video/corruption_detection/evaluation/webrtc_picture_pair_provider.cc +++ b/video/corruption_detection/evaluation/webrtc_picture_pair_provider.cc
@@ -352,13 +352,6 @@ return EncodedImageCallback::Result(EncodedImageCallback::Result::Error::OK); } -void WebRtcEncoderDecoderPicturePairProvider::OnDroppedFrame( - DropReason reason) { - RTC_DCHECK_RUN_ON(&sequence_checker_); - - encoded_image_.reset(); -} - void WebRtcEncoderDecoderPicturePairProvider::OnFrameDropped( uint32_t rtp_timestamp, int spatial_id,
diff --git a/video/corruption_detection/evaluation/webrtc_picture_pair_provider.h b/video/corruption_detection/evaluation/webrtc_picture_pair_provider.h index ea5cd37..a3c39ae 100644 --- a/video/corruption_detection/evaluation/webrtc_picture_pair_provider.h +++ b/video/corruption_detection/evaluation/webrtc_picture_pair_provider.h
@@ -69,7 +69,6 @@ // Overridden from EncodedImageCallback. Result OnEncodedImage(const EncodedImage& encoded_image, const CodecSpecificInfo*) override; - void OnDroppedFrame(DropReason reason) override; void OnFrameDropped(uint32_t rtp_timestamp, int spatial_id, bool is_end_of_temporal_unit) override;
diff --git a/video/frame_dumping_encoder.cc b/video/frame_dumping_encoder.cc index fa98174..1513e3f 100644 --- a/video/frame_dumping_encoder.cc +++ b/video/frame_dumping_encoder.cc
@@ -95,9 +95,6 @@ } return callback_->OnEncodedImage(encoded_image, codec_specific_info); } - void OnDroppedFrame(DropReason reason) override { - callback_->OnDroppedFrame(reason); - } void OnFrameDropped(uint32_t rtp_timestamp, int spatial_id, bool is_end_of_temporal_unit) override {
diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index c4ff0bd..d28ccdb 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc
@@ -295,9 +295,6 @@ return callback_->OnEncodedImage(encoded_image, codec_specific_info); } - void OnDroppedFrame(DropReason reason) override { - callback_->OnDroppedFrame(reason); - } void OnFrameDropped(uint32_t rtp_timestamp, int spatial_id, bool is_end_of_temporal_unit) override {
diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index f1305b5..1120e92 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc
@@ -972,11 +972,6 @@ return rtp_video_sender_->OnEncodedImage(encoded_image, codec_specific_info); } -void VideoSendStreamImpl::OnDroppedFrame( - EncodedImageCallback::DropReason reason) { - activity_ = true; -} - void VideoSendStreamImpl::OnFrameDropped(uint32_t rtp_timestamp, int spatial_id, bool is_end_of_temporal_unit) {
diff --git a/video/video_send_stream_impl.h b/video/video_send_stream_impl.h index dd2d2db..a06272b 100644 --- a/video/video_send_stream_impl.h +++ b/video/video_send_stream_impl.h
@@ -188,7 +188,6 @@ const CodecSpecificInfo* codec_specific_info) override; // Implements EncodedImageCallback. - void OnDroppedFrame(EncodedImageCallback::DropReason reason) override; void OnFrameDropped(uint32_t rtp_timestamp, int spatial_id, bool is_end_of_temporal_unit) override;
diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index ebef8fa..e39dc3e 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc
@@ -1281,26 +1281,6 @@ vss_impl->Stop(); } -TEST_F(VideoSendStreamImplTest, KeepAliveOnDroppedFrame) { - auto vss_impl = CreateVideoSendStreamImpl(TestVideoEncoderConfig()); - EXPECT_CALL(bitrate_allocator_, RemoveObserver(vss_impl.get())).Times(0); - vss_impl->Start(); - const uint32_t kBitrateBps = 100000; - EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps()) - .Times(1) - .WillOnce(Return(kBitrateBps)); - static_cast<BitrateAllocatorObserver*>(vss_impl.get()) - ->OnBitrateUpdated(CreateAllocation(kBitrateBps)); - encoder_queue_->PostTask([&] { - // Keep the stream from deallocating by dropping a frame. - static_cast<EncodedImageCallback*>(vss_impl.get()) - ->OnDroppedFrame(EncodedImageCallback::DropReason::kDroppedByEncoder); - }); - time_controller_.AdvanceTime(TimeDelta::Seconds(2)); - testing::Mock::VerifyAndClearExpectations(&bitrate_allocator_); - vss_impl->Stop(); -} - TEST_F(VideoSendStreamImplTest, KeepAliveOnFrameDropped) { auto vss_impl = CreateVideoSendStreamImpl(TestVideoEncoderConfig()); EXPECT_CALL(bitrate_allocator_, RemoveObserver(vss_impl.get())).Times(0);
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index dccfa5b..f107466 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc
@@ -138,17 +138,6 @@ } } -std::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 std::nullopt; - } -} bool RequiresEncoderReset(const VideoCodec& prev_send_codec, const VideoCodec& new_send_codec, @@ -2445,24 +2434,6 @@ return result; } -void VideoStreamEncoder::OnDroppedFrame(DropReason reason) { - sink_->OnDroppedFrame(reason); - encoder_queue_->PostTask([this, reason] { - RTC_DCHECK_RUN_ON(encoder_queue_.get()); - switch (reason) { - case webrtc::EncodedImageCallback::DropReason::kDroppedByEncoder: - stream_resource_manager_.OnFrameDropped( - VideoStreamEncoderObserver::DropReason::kEncoder); - break; - case webrtc::EncodedImageCallback::DropReason:: - kDroppedByMediaOptimizations: - stream_resource_manager_.OnFrameDropped( - VideoStreamEncoderObserver::DropReason::kMediaOptimization); - break; - } - }); -} - void VideoStreamEncoder::OnFrameDropped(uint32_t rtp_timestamp, int spatial_id, bool is_end_of_temporal_unit) { @@ -2476,6 +2447,8 @@ if (frame_instrumentation_generator_ && is_end_of_temporal_unit) { frame_instrumentation_generator_->OnFrameReleased(rtp_timestamp); } + encoder_stats_observer_->OnFrameDropped( + VideoStreamEncoderObserver::DropReason::kEncoder); }); } @@ -2769,9 +2742,7 @@ 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); - } + stream_resource_manager_.OnFrameDropped(reason); encoder_stats_observer_->OnFrameDropped(reason); }
diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 04e6046..9fb6b71 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h
@@ -263,7 +263,6 @@ const EncodedImage& encoded_image, const CodecSpecificInfo* codec_specific_info) override; - void OnDroppedFrame(EncodedImageCallback::DropReason reason) override; void OnFrameDropped(uint32_t rtp_timestamp, int spatial_id, bool is_end_of_temporal_unit) override;
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index eccd356..205ac6c 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc
@@ -1338,8 +1338,9 @@ num_encodes_++; if (drop_frames_) { if (encoded_image_callback_) { - encoded_image_callback_->OnDroppedFrame( - EncodedImageCallback::DropReason::kDroppedByEncoder); + encoded_image_callback_->OnFrameDropped( + input_image.rtp_timestamp(), /*spatial_id=*/0, + /*is_end_of_temporal_unit=*/true); } return WEBRTC_VIDEO_CODEC_OK; }