Make dependencies optional in VideoFrameMetadata which allows the W3C API to make them optional based on the presence of the DD (or GFD) extension as described in https://w3c.github.io/webrtc-encoded-transform/#dom-rtcencodedvideoframemetadata-dependencies This changes the signature from std::span<const int64_t> GetFrameDependencies() const; which will soon be deprecated to std::optional<std::span<const int64_t>> GetDependencies() const; The same applies to the setter. Bug: webrtc:515776877 Change-Id: I113134361943bd28593afd56dae2fce9550c9951 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/475000 Reviewed-by: Harald Alvestrand <hta@webrtc.org> Reviewed-by: Guido Urdaneta <guidou@webrtc.org> Commit-Queue: Philipp Hancke <philipp.hancke@googlemail.com> Cr-Commit-Position: refs/heads/main@{#47802}
diff --git a/api/video/video_frame_metadata.cc b/api/video/video_frame_metadata.cc index 4441b98..af9374b 100644 --- a/api/video/video_frame_metadata.cc +++ b/api/video/video_frame_metadata.cc
@@ -91,13 +91,27 @@ } std::span<const int64_t> VideoFrameMetadata::GetFrameDependencies() const { - return frame_dependencies_; + return GetDependencies().value_or(std::span<const int64_t>()); } void VideoFrameMetadata::SetFrameDependencies( std::span<const int64_t> frame_dependencies) { - frame_dependencies_.assign(frame_dependencies.begin(), - frame_dependencies.end()); + frame_dependencies_.emplace(frame_dependencies.begin(), + frame_dependencies.end()); +} + +std::optional<std::span<const int64_t>> VideoFrameMetadata::GetDependencies() + const { + return frame_dependencies_; +} + +void VideoFrameMetadata::SetDependencies( + std::optional<std::span<const int64_t>> dependencies) { + if (!dependencies.has_value()) { + frame_dependencies_.reset(); + return; + } + frame_dependencies_.emplace(dependencies->begin(), dependencies->end()); } std::span<const DecodeTargetIndication>
diff --git a/api/video/video_frame_metadata.h b/api/video/video_frame_metadata.h index 1dfd1bb..d959e82 100644 --- a/api/video/video_frame_metadata.h +++ b/api/video/video_frame_metadata.h
@@ -67,8 +67,14 @@ int GetTemporalIndex() const; void SetTemporalIndex(int temporal_index); + // TODO: https://issues.webrtc.org/515776877 - Deprecate and remove this + // method. std::span<const int64_t> GetFrameDependencies() const; + // TODO: https://issues.webrtc.org/515776877 - Deprecate and remove this + // method. void SetFrameDependencies(std::span<const int64_t> frame_dependencies); + std::optional<std::span<const int64_t>> GetDependencies() const; + void SetDependencies(std::optional<std::span<const int64_t>> dependencies); std::span<const DecodeTargetIndication> GetDecodeTargetIndications() const; void SetDecodeTargetIndications( @@ -109,7 +115,7 @@ std::optional<int64_t> frame_id_; int spatial_index_ = 0; int temporal_index_ = 0; - absl::InlinedVector<int64_t, 5> frame_dependencies_; + std::optional<absl::InlinedVector<int64_t, 5>> frame_dependencies_; absl::InlinedVector<DecodeTargetIndication, 10> decode_target_indications_; bool is_last_frame_in_picture_ = true;
diff --git a/api/video/video_frame_metadata_unittest.cc b/api/video/video_frame_metadata_unittest.cc index d12ff1f..b04e68a 100644 --- a/api/video/video_frame_metadata_unittest.cc +++ b/api/video/video_frame_metadata_unittest.cc
@@ -10,13 +10,21 @@ #include "api/video/video_frame_metadata.h" +#include <cstdint> +#include <optional> + #include "modules/video_coding/codecs/h264/include/h264_globals.h" #include "modules/video_coding/codecs/vp9/include/vp9_globals.h" +#include "test/gmock.h" #include "test/gtest.h" namespace webrtc { namespace { +using ::testing::ElementsAre; +using ::testing::IsEmpty; +using ::testing::Optional; + RTPVideoHeaderH264 ExampleHeaderH264() { NaluInfo nalu_info; nalu_info.type = 1; @@ -116,5 +124,26 @@ EXPECT_TRUE(metadata_lhs != metadata_rhs); } +TEST(VideoFrameMetadataTest, FrameDependencies) { + VideoFrameMetadata metadata; + EXPECT_EQ(metadata.GetFrameId(), std::nullopt); + EXPECT_EQ(metadata.GetDependencies(), std::nullopt); + EXPECT_THAT(metadata.GetFrameDependencies(), IsEmpty()); + + metadata.SetFrameId(42); + EXPECT_THAT(metadata.GetFrameId(), Optional(42)); + + const int64_t deps[] = {1, 2, 3}; + metadata.SetDependencies(deps); + EXPECT_THAT(metadata.GetDependencies(), Optional(ElementsAre(1, 2, 3))); + EXPECT_THAT(metadata.GetFrameDependencies(), ElementsAre(1, 2, 3)); + const int64_t other_deps[] = {4, 5}; + metadata.SetFrameDependencies(other_deps); + EXPECT_THAT(metadata.GetDependencies(), Optional(ElementsAre(4, 5))); + + metadata.SetDependencies(std::nullopt); + EXPECT_EQ(metadata.GetDependencies(), std::nullopt); +} + } // namespace } // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index 9459546..f743614 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
@@ -86,6 +86,7 @@ using ::testing::IsEmpty; using ::testing::NiceMock; using ::testing::Not; +using ::testing::Optional; using ::testing::ReturnArg; using ::testing::SaveArg; using ::testing::SizeIs; @@ -1826,7 +1827,7 @@ EXPECT_EQ(metadata.GetFrameId(), 10); EXPECT_EQ(metadata.GetTemporalIndex(), 3); EXPECT_EQ(metadata.GetSpatialIndex(), 2); - EXPECT_THAT(metadata.GetFrameDependencies(), ElementsAre(5)); + EXPECT_THAT(metadata.GetDependencies(), Optional(ElementsAre(5))); EXPECT_THAT(metadata.GetDecodeTargetIndications(), ElementsAre(DecodeTargetIndication::kSwitch)); });
diff --git a/modules/rtp_rtcp/source/rtp_video_header.cc b/modules/rtp_rtcp/source/rtp_video_header.cc index fd69e6a..cf080f5 100644 --- a/modules/rtp_rtcp/source/rtp_video_header.cc +++ b/modules/rtp_rtcp/source/rtp_video_header.cc
@@ -48,7 +48,7 @@ metadata.SetFrameId(generic->frame_id); metadata.SetSpatialIndex(generic->spatial_index); metadata.SetTemporalIndex(generic->temporal_index); - metadata.SetFrameDependencies(generic->dependencies); + metadata.SetDependencies(generic->dependencies); metadata.SetDecodeTargetIndications(generic->decode_target_indications); } metadata.SetIsLastFrameInPicture(is_last_frame_in_picture); @@ -89,8 +89,9 @@ generic->frame_id = metadata.GetFrameId().value(); generic->spatial_index = metadata.GetSpatialIndex(); generic->temporal_index = metadata.GetTemporalIndex(); - generic->dependencies.assign(metadata.GetFrameDependencies().begin(), - metadata.GetFrameDependencies().end()); + if (auto deps = metadata.GetDependencies()) { + generic->dependencies.assign(deps->begin(), deps->end()); + } generic->decode_target_indications.assign( metadata.GetDecodeTargetIndications().begin(), metadata.GetDecodeTargetIndications().end());
diff --git a/modules/rtp_rtcp/source/rtp_video_header_unittest.cc b/modules/rtp_rtcp/source/rtp_video_header_unittest.cc index b2e5bde..4a7742c 100644 --- a/modules/rtp_rtcp/source/rtp_video_header_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_video_header_unittest.cc
@@ -31,6 +31,7 @@ using ::testing::ElementsAre; using ::testing::IsEmpty; +using ::testing::Optional; TEST(RTPVideoHeaderTest, FrameType_GetAsMetadata) { RTPVideoHeader video_header; @@ -189,21 +190,21 @@ video_header.generic.emplace(); generic.dependencies = {5, 6, 7}; VideoFrameMetadata metadata = video_header.GetAsMetadata(); - EXPECT_THAT(metadata.GetFrameDependencies(), ElementsAre(5, 6, 7)); + EXPECT_THAT(metadata.GetDependencies(), Optional(ElementsAre(5, 6, 7))); } TEST(RTPVideoHeaderTest, FrameDependency_GetAsMetadataWhenGenericIsMissing) { RTPVideoHeader video_header; VideoFrameMetadata metadata = video_header.GetAsMetadata(); ASSERT_FALSE(video_header.generic); - EXPECT_THAT(metadata.GetFrameDependencies(), IsEmpty()); + EXPECT_EQ(metadata.GetDependencies(), std::nullopt); } TEST(RTPVideoHeaderTest, FrameDependencies_FromMetadata) { VideoFrameMetadata metadata; absl::InlinedVector<int64_t, 5> dependencies = {5, 6, 7}; metadata.SetFrameId(123); // Must have a frame ID for related properties. - metadata.SetFrameDependencies(dependencies); + metadata.SetDependencies(dependencies); RTPVideoHeader video_header = RTPVideoHeader::FromMetadata(metadata); EXPECT_TRUE(video_header.generic.has_value()); EXPECT_THAT(video_header.generic->dependencies, ElementsAre(5, 6, 7));
diff --git a/modules/rtp_rtcp/source/rtp_video_stream_receiver_frame_transformer_delegate.cc b/modules/rtp_rtcp/source/rtp_video_stream_receiver_frame_transformer_delegate.cc index 8bb3561..fdb4773 100644 --- a/modules/rtp_rtcp/source/rtp_video_stream_receiver_frame_transformer_delegate.cc +++ b/modules/rtp_rtcp/source/rtp_video_stream_receiver_frame_transformer_delegate.cc
@@ -79,7 +79,7 @@ // dependencies. VideoFrameMetadata new_metadata = Metadata(); new_metadata.SetFrameId(metadata.GetFrameId()); - new_metadata.SetFrameDependencies(metadata.GetFrameDependencies()); + new_metadata.SetDependencies(metadata.GetDependencies()); RTC_DCHECK(new_metadata == metadata) << "TransformableVideoReceiverFrame::SetMetadata can be only used to " "change frameID and dependencies";
diff --git a/modules/rtp_rtcp/source/rtp_video_stream_receiver_frame_transformer_delegate_unittest.cc b/modules/rtp_rtcp/source/rtp_video_stream_receiver_frame_transformer_delegate_unittest.cc index 7e63a84..03b2821 100644 --- a/modules/rtp_rtcp/source/rtp_video_stream_receiver_frame_transformer_delegate_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_video_stream_receiver_frame_transformer_delegate_unittest.cc
@@ -52,6 +52,7 @@ using ::testing::ElementsAre; using ::testing::NiceMock; using ::testing::NotNull; +using ::testing::Optional; using ::testing::Return; using ::testing::SaveArg; @@ -215,7 +216,7 @@ EXPECT_EQ(metadata.GetFrameId(), 10); EXPECT_EQ(metadata.GetTemporalIndex(), 3); EXPECT_EQ(metadata.GetSpatialIndex(), 2); - EXPECT_THAT(metadata.GetFrameDependencies(), ElementsAre(5)); + EXPECT_THAT(metadata.GetDependencies(), Optional(ElementsAre(5))); EXPECT_THAT(metadata.GetDecodeTargetIndications(), ElementsAre(DecodeTargetIndication::kSwitch)); EXPECT_EQ(metadata.GetCsrcs(), csrcs); @@ -313,11 +314,11 @@ *transformable_frame); VideoFrameMetadata metadata = video_frame.Metadata(); EXPECT_EQ(metadata.GetFrameId(), 10); - EXPECT_THAT(metadata.GetFrameDependencies(), ElementsAre(5)); + EXPECT_THAT(metadata.GetDependencies(), Optional(ElementsAre(5))); EXPECT_EQ(metadata.GetCsrcs(), csrcs); metadata.SetFrameId(20); - metadata.SetFrameDependencies(std::vector<int64_t>{15}); + metadata.SetDependencies(std::vector<int64_t>{15}); video_frame.SetMetadata(metadata); callback->OnTransformedFrame(std::move(transformable_frame)); });