From 6e072e6bfb51ab9ee9aa640a3d46d602b2c97454 Mon Sep 17 00:00:00 2001 From: Fanny Linderborg Date: Tue, 3 Sep 2024 10:34:52 +0200 Subject: [PATCH] Rename is_key_frame to communicate_upper_bits in FrameInstrumentation*Data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is to make it clear that this field indicate whether the upper bits of the sequence number should be communicated. However, the current implementation only sets the field if it is a key frame. Bug: webrtc:358039777 Change-Id: Ic2c8b6d91499e4e5cf25b8ce9591d326d7044fb0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361402 Commit-Queue: Fanny Linderborg Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/main@{#42924} --- common_video/frame_instrumentation_data.h | 4 ++-- .../frame_instrumentation_generator.cc | 4 ++-- ...rame_instrumentation_generator_unittest.cc | 24 ++++++++++++------- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/common_video/frame_instrumentation_data.h b/common_video/frame_instrumentation_data.h index e6d268a5e6..49a5e66787 100644 --- a/common_video/frame_instrumentation_data.h +++ b/common_video/frame_instrumentation_data.h @@ -18,12 +18,12 @@ namespace webrtc { // TODO: b/358039777 - Error handling: negative values etc. struct FrameInstrumentationSyncData { int sequence_index; - bool is_key_frame; + bool communicate_upper_bits; }; struct FrameInstrumentationData { int sequence_index; - bool is_key_frame; + bool communicate_upper_bits; double std_dev; int luma_error_threshold; int chroma_error_threshold; diff --git a/video/corruption_detection/frame_instrumentation_generator.cc b/video/corruption_detection/frame_instrumentation_generator.cc index e14782dcd0..b5e2869548 100644 --- a/video/corruption_detection/frame_instrumentation_generator.cc +++ b/video/corruption_detection/frame_instrumentation_generator.cc @@ -132,7 +132,7 @@ FrameInstrumentationGenerator::OnEncodedImage( return std::nullopt; } return FrameInstrumentationSyncData{.sequence_index = sequence_index, - .is_key_frame = true}; + .communicate_upper_bits = true}; } std::optional filter_settings = @@ -153,7 +153,7 @@ FrameInstrumentationGenerator::OnEncodedImage( FrameInstrumentationData data = { .sequence_index = sequence_index, - .is_key_frame = is_key_frame, + .communicate_upper_bits = is_key_frame, .std_dev = filter_settings->std_dev, .luma_error_threshold = filter_settings->luma_error_threshold, .chroma_error_threshold = filter_settings->chroma_error_threshold}; diff --git a/video/corruption_detection/frame_instrumentation_generator_unittest.cc b/video/corruption_detection/frame_instrumentation_generator_unittest.cc index 146a3f7840..8bdb9a7b4d 100644 --- a/video/corruption_detection/frame_instrumentation_generator_unittest.cc +++ b/video/corruption_detection/frame_instrumentation_generator_unittest.cc @@ -152,7 +152,7 @@ TEST(FrameInstrumentationGeneratorTest, FrameInstrumentationData frame_instrumentation_data = absl::get(*data); EXPECT_EQ(frame_instrumentation_data.sequence_index, 0); - EXPECT_TRUE(frame_instrumentation_data.is_key_frame); + EXPECT_TRUE(frame_instrumentation_data.communicate_upper_bits); EXPECT_NE(frame_instrumentation_data.std_dev, 0.0); EXPECT_NE(frame_instrumentation_data.luma_error_threshold, 0); EXPECT_NE(frame_instrumentation_data.chroma_error_threshold, 0); @@ -192,7 +192,7 @@ TEST(FrameInstrumentationGeneratorTest, FrameInstrumentationData frame_instrumentation_data = absl::get(*data); EXPECT_EQ(frame_instrumentation_data.sequence_index, 0); - EXPECT_TRUE(frame_instrumentation_data.is_key_frame); + EXPECT_TRUE(frame_instrumentation_data.communicate_upper_bits); EXPECT_NE(frame_instrumentation_data.std_dev, 0.0); EXPECT_NE(frame_instrumentation_data.luma_error_threshold, 0); EXPECT_NE(frame_instrumentation_data.chroma_error_threshold, 0); @@ -234,7 +234,7 @@ TEST(FrameInstrumentationGeneratorTest, FrameInstrumentationData frame_instrumentation_data = absl::get(*data); EXPECT_EQ(frame_instrumentation_data.sequence_index, 0); - EXPECT_TRUE(frame_instrumentation_data.is_key_frame); + EXPECT_TRUE(frame_instrumentation_data.communicate_upper_bits); EXPECT_NE(frame_instrumentation_data.std_dev, 0.0); EXPECT_NE(frame_instrumentation_data.luma_error_threshold, 0); EXPECT_NE(frame_instrumentation_data.chroma_error_threshold, 0); @@ -322,8 +322,10 @@ TEST(FrameInstrumentationGeneratorTest, ASSERT_TRUE(absl::holds_alternative(*data2)); - EXPECT_TRUE(absl::get(*data1).is_key_frame); - EXPECT_TRUE(absl::get(*data2).is_key_frame); + EXPECT_TRUE( + absl::get(*data1).communicate_upper_bits); + EXPECT_TRUE( + absl::get(*data2).communicate_upper_bits); } } @@ -373,16 +375,20 @@ TEST(FrameInstrumentationGeneratorTest, ASSERT_TRUE(absl::holds_alternative(*data2)); - EXPECT_TRUE(absl::get(*data1).is_key_frame); - EXPECT_TRUE(absl::get(*data2).is_key_frame); + EXPECT_TRUE( + absl::get(*data1).communicate_upper_bits); + EXPECT_TRUE( + absl::get(*data2).communicate_upper_bits); } else if (data1.has_value() || data2.has_value()) { if (data1.has_value()) { ASSERT_TRUE(absl::holds_alternative(*data1)); - EXPECT_FALSE(absl::get(*data1).is_key_frame); + EXPECT_FALSE( + absl::get(*data1).communicate_upper_bits); } if (data2.has_value()) { ASSERT_TRUE(absl::holds_alternative(*data2)); - EXPECT_FALSE(absl::get(*data2).is_key_frame); + EXPECT_FALSE( + absl::get(*data2).communicate_upper_bits); } has_found_delta_frame = true; }