From 06717773a58a754163fb44638405ef61d59833ca Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 21 Aug 2023 18:17:31 +0200 Subject: [PATCH] Move EncodedImage::playout_delay_ to private section of the class Remove code where integer -1 as delay is used to represent unset value. Bug: webrtc:13756 Change-Id: I16a01e12c25a09ce21a971c9edabf47af5936662 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/316923 Auto-Submit: Danil Chapovalov Commit-Queue: Philip Eliasson Reviewed-by: Philip Eliasson Cr-Commit-Position: refs/heads/main@{#40592} --- api/video/encoded_image.h | 23 +++++-------------- call/rtp_payload_params.cc | 2 +- .../source/rtp_rtcp_impl2_unittest.cc | 1 - .../rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 1 - modules/video_coding/deprecated/packet.cc | 3 +-- video/rtp_video_stream_receiver2_unittest.cc | 2 +- 6 files changed, 9 insertions(+), 23 deletions(-) diff --git a/api/video/encoded_image.h b/api/video/encoded_image.h index 244a438dd8..c1f14475cd 100644 --- a/api/video/encoded_image.h +++ b/api/video/encoded_image.h @@ -139,19 +139,11 @@ class RTC_EXPORT EncodedImage { } absl::optional PlayoutDelay() const { - if (playout_delay_.Valid()) { - return playout_delay_; - } - return absl::nullopt; + return playout_delay_; } void SetPlayoutDelay(absl::optional playout_delay) { - if (playout_delay.has_value()) { - playout_delay_ = *playout_delay; - } else { - playout_delay_.min_ms = -1; - playout_delay_.max_ms = -1; - } + playout_delay_ = playout_delay; } // These methods along with the private member video_frame_tracking_id_ are @@ -220,13 +212,6 @@ class RTC_EXPORT EncodedImage { VideoContentType content_type_ = VideoContentType::UNSPECIFIED; int qp_ = -1; // Quantizer value. - // When an application indicates non-zero values here, it is taken as an - // indication that all future frames will be constrained with those limits - // until the application indicates a change again. - // TODO(bugs.webrc.org/13756): Make private and represent unset value with - // optional. - VideoPlayoutDelay playout_delay_; - struct Timing { uint8_t flags = VideoSendTiming::kInvalid; int64_t encode_start_ms = 0; @@ -242,6 +227,10 @@ class RTC_EXPORT EncodedImage { private: size_t capacity() const { return encoded_data_ ? encoded_data_->size() : 0; } + // When set, indicates that all future frames will be constrained with those + // limits until the application indicates a change again. + absl::optional playout_delay_; + rtc::scoped_refptr encoded_data_; size_t size_ = 0; // Size of encoded frame data. uint32_t timestamp_rtp_ = 0; diff --git a/call/rtp_payload_params.cc b/call/rtp_payload_params.cc index e9bfb30ae3..f4b09ce913 100644 --- a/call/rtp_payload_params.cc +++ b/call/rtp_payload_params.cc @@ -207,7 +207,7 @@ RTPVideoHeader RtpPayloadParams::GetRtpVideoHeader( rtp_video_header.frame_type = image._frameType; rtp_video_header.rotation = image.rotation_; rtp_video_header.content_type = image.content_type_; - rtp_video_header.playout_delay = image.playout_delay_; + rtp_video_header.playout_delay = image.PlayoutDelay(); rtp_video_header.width = image._encodedWidth; rtp_video_header.height = image._encodedHeight; rtp_video_header.color_space = image.ColorSpace() diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc index cd4b8f3ecc..0821f6deb0 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc @@ -349,7 +349,6 @@ class RtpRtcpImpl2Test : public ::testing::Test { rtp_video_header.height = kHeight; rtp_video_header.rotation = kVideoRotation_0; rtp_video_header.content_type = VideoContentType::UNSPECIFIED; - rtp_video_header.playout_delay = {-1, -1}; rtp_video_header.is_first_packet_in_frame = true; rtp_video_header.simulcastIdx = 0; rtp_video_header.codec = kVideoCodecVP8; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 8a71a199fc..abf3b639f8 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -222,7 +222,6 @@ class RtpRtcpImplTest : public ::testing::Test { rtp_video_header.height = kHeight; rtp_video_header.rotation = kVideoRotation_0; rtp_video_header.content_type = VideoContentType::UNSPECIFIED; - rtp_video_header.playout_delay = {-1, -1}; rtp_video_header.is_first_packet_in_frame = true; rtp_video_header.simulcastIdx = 0; rtp_video_header.codec = kVideoCodecVP8; diff --git a/modules/video_coding/deprecated/packet.cc b/modules/video_coding/deprecated/packet.cc index 7099ccfd9d..110f38e0fc 100644 --- a/modules/video_coding/deprecated/packet.cc +++ b/modules/video_coding/deprecated/packet.cc @@ -26,7 +26,6 @@ VCMPacket::VCMPacket() completeNALU(kNaluUnset), insertStartCode(false), video_header() { - video_header.playout_delay = {-1, -1}; } VCMPacket::VCMPacket(const uint8_t* ptr, @@ -60,7 +59,7 @@ VCMPacket::VCMPacket(const uint8_t* ptr, // Playout decisions are made entirely based on first packet in a frame. if (!is_first_packet_in_frame()) { - video_header.playout_delay = {-1, -1}; + video_header.playout_delay = absl::nullopt; } } diff --git a/video/rtp_video_stream_receiver2_unittest.cc b/video/rtp_video_stream_receiver2_unittest.cc index 4e10d42bb1..9085863922 100644 --- a/video/rtp_video_stream_receiver2_unittest.cc +++ b/video/rtp_video_stream_receiver2_unittest.cc @@ -1224,7 +1224,7 @@ TEST_P(RtpVideoStreamReceiver2TestPlayoutDelay, PlayoutDelay) { EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(_)) .WillOnce(Invoke([expected_playout_delay = GetParam().expected_delay](EncodedFrame* frame) { - EXPECT_EQ(frame->EncodedImage().playout_delay_, expected_playout_delay); + EXPECT_EQ(frame->EncodedImage().PlayoutDelay(), expected_playout_delay); })); rtp_video_stream_receiver_->OnReceivedPayloadData( received_packet.PayloadBuffer(), received_packet, video_header);