Ensure frame type is copied over for cloned sender frames
Previously cloned frames ended up with the metadata saying it was a
delta frame, even for keyframes.
Bug: chromium:1425362
Change-Id: I7a9438f124b75f6be9a5705d20fa65b2f7179a22
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298020
Commit-Queue: Tony Herre <herre@google.com>
Auto-Submit: Tony Herre <herre@google.com>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39588}
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc b/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc
index 7ced8dd..3b26214 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate.cc
@@ -216,9 +216,13 @@
original->GetData().data(), original->GetData().size());
EncodedImage encoded_image;
encoded_image.SetEncodedData(encoded_image_buffer);
+ encoded_image._frameType = original->IsKeyFrame()
+ ? VideoFrameType::kVideoFrameKey
+ : VideoFrameType::kVideoFrameDelta;
+ // TODO(bugs.webrtc.org/14708): Fill in other EncodedImage parameters
+
VideoFrameMetadata metadata = original->Metadata();
RTPVideoHeader new_header = RTPVideoHeader::FromMetadata(metadata);
- // TODO(bugs.webrtc.org/14708): Fill in other EncodedImage parameters
return std::make_unique<TransformableVideoSenderFrame>(
encoded_image, new_header, original->GetPayloadType(), new_header.codec,
original->GetTimestamp(),
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate_unittest.cc
index 075f680..ad1d0b3 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video_frame_transformer_delegate_unittest.cc
@@ -58,9 +58,12 @@
~RtpSenderVideoFrameTransformerDelegateTest() override = default;
std::unique_ptr<TransformableFrameInterface> GetTransformableFrame(
- rtc::scoped_refptr<RTPSenderVideoFrameTransformerDelegate> delegate) {
+ rtc::scoped_refptr<RTPSenderVideoFrameTransformerDelegate> delegate,
+ bool key_frame = false) {
EncodedImage encoded_image;
encoded_image.SetEncodedData(EncodedImageBuffer::Create(1));
+ encoded_image._frameType = key_frame ? VideoFrameType::kVideoFrameKey
+ : VideoFrameType::kVideoFrameDelta;
std::unique_ptr<TransformableFrameInterface> frame = nullptr;
EXPECT_CALL(*frame_transformer_, Transform)
.WillOnce([&](std::unique_ptr<TransformableFrameInterface>
@@ -154,16 +157,36 @@
GetTransformableFrame(delegate);
ASSERT_TRUE(frame);
- TransformableVideoFrameInterface* video_frame =
- static_cast<TransformableVideoFrameInterface*>(frame.get());
+ auto& video_frame = static_cast<TransformableVideoFrameInterface&>(*frame);
std::unique_ptr<TransformableVideoFrameInterface> clone =
- CloneSenderVideoFrame(video_frame);
+ CloneSenderVideoFrame(&video_frame);
- EXPECT_EQ(video_frame->IsKeyFrame(), clone->IsKeyFrame());
- EXPECT_EQ(video_frame->GetPayloadType(), clone->GetPayloadType());
- EXPECT_EQ(video_frame->GetSsrc(), clone->GetSsrc());
- EXPECT_EQ(video_frame->GetTimestamp(), clone->GetTimestamp());
- EXPECT_EQ(video_frame->Metadata(), clone->Metadata());
+ EXPECT_EQ(clone->IsKeyFrame(), video_frame.IsKeyFrame());
+ EXPECT_EQ(clone->GetPayloadType(), video_frame.GetPayloadType());
+ EXPECT_EQ(clone->GetSsrc(), video_frame.GetSsrc());
+ EXPECT_EQ(clone->GetTimestamp(), video_frame.GetTimestamp());
+ EXPECT_EQ(clone->Metadata(), video_frame.Metadata());
+}
+
+TEST_F(RtpSenderVideoFrameTransformerDelegateTest, CloneKeyFrame) {
+ auto delegate = rtc::make_ref_counted<RTPSenderVideoFrameTransformerDelegate>(
+ &test_sender_, frame_transformer_,
+ /*ssrc=*/1111, /*csrcs=*/std::vector<uint32_t>(),
+ time_controller_.CreateTaskQueueFactory().get());
+
+ std::unique_ptr<TransformableFrameInterface> frame =
+ GetTransformableFrame(delegate, /*key_frame=*/true);
+ ASSERT_TRUE(frame);
+
+ auto& video_frame = static_cast<TransformableVideoFrameInterface&>(*frame);
+ std::unique_ptr<TransformableVideoFrameInterface> clone =
+ CloneSenderVideoFrame(&video_frame);
+
+ EXPECT_EQ(clone->IsKeyFrame(), video_frame.IsKeyFrame());
+ EXPECT_EQ(clone->GetPayloadType(), video_frame.GetPayloadType());
+ EXPECT_EQ(clone->GetSsrc(), video_frame.GetSsrc());
+ EXPECT_EQ(clone->GetTimestamp(), video_frame.GetTimestamp());
+ EXPECT_EQ(clone->Metadata(), video_frame.Metadata());
}
TEST_F(RtpSenderVideoFrameTransformerDelegateTest, MetadataAfterSetMetadata) {
@@ -175,8 +198,7 @@
std::unique_ptr<TransformableFrameInterface> frame =
GetTransformableFrame(delegate);
ASSERT_TRUE(frame);
- TransformableVideoFrameInterface* video_frame =
- static_cast<TransformableVideoFrameInterface*>(frame.get());
+ auto& video_frame = static_cast<TransformableVideoFrameInterface&>(*frame);
VideoFrameMetadata metadata;
metadata.SetFrameType(VideoFrameType::kVideoFrameKey);
@@ -184,8 +206,8 @@
metadata.SetSsrc(2222);
metadata.SetCsrcs({1, 2, 3});
- video_frame->SetMetadata(metadata);
- VideoFrameMetadata actual_metadata = video_frame->Metadata();
+ video_frame.SetMetadata(metadata);
+ VideoFrameMetadata actual_metadata = video_frame.Metadata();
// TODO(bugs.webrtc.org/14708): Just EXPECT_EQ the whole Metadata once the
// equality operator lands.
