From f045dbd67cbcaaf45e19970492b62edb0fc632d9 Mon Sep 17 00:00:00 2001 From: Fanny Linderborg Date: Wed, 18 Sep 2024 10:50:38 +0200 Subject: [PATCH] Modify sequence index on key frames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For key frames: increase the sequence index until the last 7 bits are all zeroes. If this results in an overflow, wraparound to 0. Also: * Allow setting and getting the sequence index * Allow getting LayerId Bug: webrtc:358039777 Change-Id: Ibe16689a3d1eb5706d4fce5c9220770046f26896 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362540 Reviewed-by: Erik Språng Auto-Submit: Fanny Linderborg Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/main@{#43042} --- video/corruption_detection/BUILD.gn | 1 + .../frame_instrumentation_generator.cc | 51 ++- .../frame_instrumentation_generator.h | 6 + ...rame_instrumentation_generator_unittest.cc | 294 +++++++++++++++++- video/corruption_detection/halton_sequence.cc | 7 + video/corruption_detection/halton_sequence.h | 2 +- 6 files changed, 351 insertions(+), 10 deletions(-) diff --git a/video/corruption_detection/BUILD.gn b/video/corruption_detection/BUILD.gn index fa6a6629fa..bbcc122711 100644 --- a/video/corruption_detection/BUILD.gn +++ b/video/corruption_detection/BUILD.gn @@ -24,6 +24,7 @@ rtc_library("frame_instrumentation_generator") { "../../common_video:frame_instrumentation_data", "../../modules:module_api_public", "../../modules/video_coding:video_coding_utility", + "../../rtc_base:checks", "../../rtc_base:logging", "//third_party/abseil-cpp/absl/algorithm:container", "//third_party/abseil-cpp/absl/types:variant", diff --git a/video/corruption_detection/frame_instrumentation_generator.cc b/video/corruption_detection/frame_instrumentation_generator.cc index b5e2869548..d0add00514 100644 --- a/video/corruption_detection/frame_instrumentation_generator.cc +++ b/video/corruption_detection/frame_instrumentation_generator.cc @@ -28,6 +28,7 @@ #include "common_video/frame_instrumentation_data.h" #include "modules/include/module_common_types_public.h" #include "modules/video_coding/utility/qp_parser.h" +#include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "video/corruption_detection/generic_mapping_functions.h" #include "video/corruption_detection/halton_frame_sampler.h" @@ -94,13 +95,11 @@ FrameInstrumentationGenerator::OnEncodedImage( } VideoFrame captured_frame = captured_frames_.front(); - int layer_id = std::max(encoded_image.SpatialIndex().value_or(0), - encoded_image.SimulcastIndex().value_or(0)); + int layer_id = GetLayerId(encoded_image); + bool is_key_frame = encoded_image.FrameType() == VideoFrameType::kVideoFrameKey; - if (is_key_frame) { - contexts_.erase(layer_id); - } else { + if (!is_key_frame) { for (const auto& [unused, context] : contexts_) { if (context.rtp_timestamp_of_last_key_frame == rtp_timestamp_encoded_image) { @@ -110,7 +109,6 @@ FrameInstrumentationGenerator::OnEncodedImage( } } } - if (is_key_frame) { contexts_[layer_id].rtp_timestamp_of_last_key_frame = encoded_image.RtpTimestamp(); @@ -121,6 +119,23 @@ FrameInstrumentationGenerator::OnEncodedImage( } int sequence_index = contexts_[layer_id].frame_sampler.GetCurrentIndex(); + bool communicate_upper_bits = false; + if (is_key_frame) { + communicate_upper_bits = true; + // Increase until all the last 7 bits are zeroes. + + // If this would overflow to 15 bits, reset to 0. + if (sequence_index > 0b0011'1111'1000'0000) { + sequence_index = 0; + } else if ((sequence_index & 0b0111'1111) != 0) { + // Last 7 bits are not all zeroes. + sequence_index >>= 7; + sequence_index += 1; + sequence_index <<= 7; + } + contexts_[layer_id].frame_sampler.SetCurrentIndex(sequence_index); + } + // TODO: b/358039777 - Maybe allow other sample sizes as well std::vector sample_coordinates = contexts_[layer_id] @@ -153,7 +168,7 @@ FrameInstrumentationGenerator::OnEncodedImage( FrameInstrumentationData data = { .sequence_index = sequence_index, - .communicate_upper_bits = is_key_frame, + .communicate_upper_bits = communicate_upper_bits, .std_dev = filter_settings->std_dev, .luma_error_threshold = filter_settings->luma_error_threshold, .chroma_error_threshold = filter_settings->chroma_error_threshold}; @@ -167,4 +182,26 @@ FrameInstrumentationGenerator::OnEncodedImage( return data; } +std::optional FrameInstrumentationGenerator::GetHaltonSequenceIndex( + int layer_id) const { + auto it = contexts_.find(layer_id); + if (it == contexts_.end()) { + return std::nullopt; + } + return it->second.frame_sampler.GetCurrentIndex(); +} + +void FrameInstrumentationGenerator::SetHaltonSequenceIndex(int index, + int layer_id) { + if (index <= 0x3FFF) { + contexts_[layer_id].frame_sampler.SetCurrentIndex(index); + } + RTC_DCHECK_LE(index, 0x3FFF) << "Index must not be larger than 0x3FFF"; +} + +int FrameInstrumentationGenerator::GetLayerId( + const EncodedImage& encoded_image) const { + return std::max(encoded_image.SpatialIndex().value_or(0), + encoded_image.SimulcastIndex().value_or(0)); +} } // namespace webrtc diff --git a/video/corruption_detection/frame_instrumentation_generator.h b/video/corruption_detection/frame_instrumentation_generator.h index b37f21f776..099c12db63 100644 --- a/video/corruption_detection/frame_instrumentation_generator.h +++ b/video/corruption_detection/frame_instrumentation_generator.h @@ -41,6 +41,12 @@ class FrameInstrumentationGenerator { absl::variant> OnEncodedImage(const EncodedImage& encoded_image); + // Returns `std::nullopt` if there is no context for the given layer. + std::optional GetHaltonSequenceIndex(int layer_id) const; + void SetHaltonSequenceIndex(int index, int layer_id); + + int GetLayerId(const EncodedImage& encoded_image) const; + private: struct Context { HaltonFrameSampler frame_sampler; diff --git a/video/corruption_detection/frame_instrumentation_generator_unittest.cc b/video/corruption_detection/frame_instrumentation_generator_unittest.cc index 8bdb9a7b4d..2305127699 100644 --- a/video/corruption_detection/frame_instrumentation_generator_unittest.cc +++ b/video/corruption_detection/frame_instrumentation_generator_unittest.cc @@ -22,10 +22,12 @@ #include "api/video/video_frame.h" #include "api/video/video_frame_type.h" #include "common_video/frame_instrumentation_data.h" +#include "test/gmock.h" #include "test/gtest.h" namespace webrtc { namespace { +using ::testing::ElementsAre; constexpr int kDefaultScaledWidth = 4; constexpr int kDefaultScaledHeight = 4; @@ -46,6 +48,22 @@ scoped_refptr MakeDefaultI420FrameBuffer() { kDefaultVContent.data(), kDefaultChromaWidth); } +scoped_refptr MakeI420FrameBufferWithDifferentPixelValues() { + // Create an I420 frame of size 4x4. + const int kDefaultLumaWidth = 4; + const int kDefaultLumaHeight = 4; + const int kDefaultChromaWidth = 2; + std::vector kDefaultYContent = {1, 2, 3, 4, 5, 6, 7, 8, + 9, 10, 11, 12, 13, 14, 15, 16}; + std::vector kDefaultUContent = {17, 18, 19, 20}; + std::vector kDefaultVContent = {21, 22, 23, 24}; + + return I420Buffer::Copy(kDefaultLumaWidth, kDefaultLumaHeight, + kDefaultYContent.data(), kDefaultLumaWidth, + kDefaultUContent.data(), kDefaultChromaWidth, + kDefaultVContent.data(), kDefaultChromaWidth); +} + TEST(FrameInstrumentationGeneratorTest, ReturnsNothingWhenNoFramesHaveBeenProvided) { FrameInstrumentationGenerator generator(VideoCodecType::kVideoCodecGeneric); @@ -305,8 +323,7 @@ TEST(FrameInstrumentationGeneratorTest, encoded_image2._encodedWidth = kDefaultScaledWidth; encoded_image2._encodedHeight = kDefaultScaledHeight; - generator.OnCapturedFrame(frame1); - generator.OnCapturedFrame(frame2); + generator.OnCapturedFrame(frame); std::optional< absl::variant> @@ -329,6 +346,70 @@ TEST(FrameInstrumentationGeneratorTest, } } +TEST(FrameInstrumentationGeneratorTest, + SvcLayersSequenceIndicesIncreaseIndependentOnEachother) { + FrameInstrumentationGenerator generator(VideoCodecType::kVideoCodecVP9); + VideoFrame frame1 = + VideoFrame::Builder() + .set_video_frame_buffer(MakeI420FrameBufferWithDifferentPixelValues()) + .set_rtp_timestamp(1) + .build(); + VideoFrame frame2 = + VideoFrame::Builder() + .set_video_frame_buffer(MakeI420FrameBufferWithDifferentPixelValues()) + .set_rtp_timestamp(2) + .build(); + for (const VideoFrame& frame : {frame1, frame2}) { + EncodedImage encoded_image1; + encoded_image1.SetRtpTimestamp(frame.rtp_timestamp()); + encoded_image1.SetFrameType(VideoFrameType::kVideoFrameKey); + encoded_image1.SetSpatialIndex(0); + encoded_image1.qp_ = 10; + encoded_image1._encodedWidth = kDefaultScaledWidth; + encoded_image1._encodedHeight = kDefaultScaledHeight; + + EncodedImage encoded_image2; + encoded_image2.SetRtpTimestamp(frame.rtp_timestamp()); + encoded_image2.SetFrameType(VideoFrameType::kVideoFrameDelta); + encoded_image2.SetSpatialIndex(1); + encoded_image2.qp_ = 10; + encoded_image2._encodedWidth = kDefaultScaledWidth; + encoded_image2._encodedHeight = kDefaultScaledHeight; + + generator.OnCapturedFrame(frame); + + std::optional< + absl::variant> + data1 = generator.OnEncodedImage(encoded_image1); + + std::optional< + absl::variant> + data2 = generator.OnEncodedImage(encoded_image2); + + ASSERT_TRUE(data1.has_value()); + ASSERT_TRUE(data2.has_value()); + ASSERT_TRUE(absl::holds_alternative(*data1)); + + ASSERT_TRUE(absl::holds_alternative(*data2)); + + FrameInstrumentationData frame_instrumentation_data1 = + absl::get(*data1); + FrameInstrumentationData frame_instrumentation_data2 = + absl::get(*data2); + + EXPECT_TRUE(frame_instrumentation_data1.communicate_upper_bits); + EXPECT_TRUE(frame_instrumentation_data2.communicate_upper_bits); + + EXPECT_EQ(frame_instrumentation_data1.sequence_index, + frame_instrumentation_data2.sequence_index); + + // In the test the frames have equal frame buffers so the sample values + // should be equal. + EXPECT_THAT(frame_instrumentation_data1.sample_values, + frame_instrumentation_data2.sample_values); + } +} + TEST(FrameInstrumentationGeneratorTest, OutputsDeltaFrameInstrumentationDataForSimulcast) { FrameInstrumentationGenerator generator(VideoCodecType::kVideoCodecVP9); @@ -396,5 +477,214 @@ TEST(FrameInstrumentationGeneratorTest, EXPECT_TRUE(has_found_delta_frame); } +TEST(FrameInstrumentationGeneratorTest, + SequenceIndexIncreasesCorrectlyAtNewKeyFrame) { + FrameInstrumentationGenerator generator(VideoCodecType::kVideoCodecVP8); + VideoFrame frame1 = + VideoFrame::Builder() + .set_video_frame_buffer(MakeI420FrameBufferWithDifferentPixelValues()) + .set_rtp_timestamp(1) + .build(); + VideoFrame frame2 = + VideoFrame::Builder() + .set_video_frame_buffer(MakeI420FrameBufferWithDifferentPixelValues()) + .set_rtp_timestamp(2) + .build(); + EncodedImage encoded_image1; + encoded_image1.SetRtpTimestamp(1); + encoded_image1.SetFrameType(VideoFrameType::kVideoFrameKey); + encoded_image1.qp_ = 10; + encoded_image1._encodedWidth = kDefaultScaledWidth; + encoded_image1._encodedHeight = kDefaultScaledHeight; + + // Delta frame that is an upper layer of an SVC key frame. + EncodedImage encoded_image2; + encoded_image2.SetRtpTimestamp(2); + encoded_image2.SetFrameType(VideoFrameType::kVideoFrameKey); + encoded_image2.qp_ = 10; + encoded_image2._encodedWidth = kDefaultScaledWidth; + encoded_image2._encodedHeight = kDefaultScaledHeight; + + generator.OnCapturedFrame(frame1); + generator.OnCapturedFrame(frame2); + + ASSERT_EQ(generator.GetLayerId(encoded_image1), + generator.GetLayerId(encoded_image2)); + generator.SetHaltonSequenceIndex(0b0010'1010, + generator.GetLayerId(encoded_image1)); + + std::optional< + absl::variant> + data1 = generator.OnEncodedImage(encoded_image1); + std::optional< + absl::variant> + data2 = generator.OnEncodedImage(encoded_image2); + + ASSERT_TRUE(data1.has_value()); + ASSERT_TRUE(data2.has_value()); + ASSERT_TRUE(absl::holds_alternative(*data1)); + ASSERT_TRUE(absl::holds_alternative(*data2)); + + FrameInstrumentationData frame_instrumentation_data1 = + absl::get(*data1); + FrameInstrumentationData frame_instrumentation_data2 = + absl::get(*data2); + + EXPECT_EQ(frame_instrumentation_data1.sequence_index, 0b0000'1000'0000); + EXPECT_EQ(frame_instrumentation_data2.sequence_index, 0b0001'0000'0000); + + EXPECT_THAT(frame_instrumentation_data1.sample_values, + ElementsAre(17, 10, 8, 24, 2, 12, 20, 13, 3, 21, 5, 15, 17)); + EXPECT_THAT(frame_instrumentation_data2.sample_values, + ElementsAre(3, 21, 6, 16, 18, 9, 7, 23, 2, 12, 20, 14, 4)); +} + +TEST(FrameInstrumentationGeneratorTest, + SequenceIndexThatWouldOverflowTo15BitsIncreasesCorrectlyAtNewKeyFrame) { + FrameInstrumentationGenerator generator(VideoCodecType::kVideoCodecVP8); + VideoFrame frame1 = + VideoFrame::Builder() + .set_video_frame_buffer(MakeI420FrameBufferWithDifferentPixelValues()) + .set_rtp_timestamp(1) + .build(); + VideoFrame frame2 = + VideoFrame::Builder() + .set_video_frame_buffer(MakeI420FrameBufferWithDifferentPixelValues()) + .set_rtp_timestamp(2) + .build(); + EncodedImage encoded_image1; + encoded_image1.SetRtpTimestamp(1); + encoded_image1.SetFrameType(VideoFrameType::kVideoFrameKey); + encoded_image1.qp_ = 10; + encoded_image1._encodedWidth = kDefaultScaledWidth; + encoded_image1._encodedHeight = kDefaultScaledHeight; + encoded_image1.SetSimulcastIndex(0); + + EncodedImage encoded_image2; + encoded_image2.SetRtpTimestamp(2); + encoded_image2.SetFrameType(VideoFrameType::kVideoFrameKey); + encoded_image2.qp_ = 10; + encoded_image2._encodedWidth = kDefaultScaledWidth; + encoded_image2._encodedHeight = kDefaultScaledHeight; + encoded_image2.SetSimulcastIndex(0); + + generator.OnCapturedFrame(frame1); + generator.OnCapturedFrame(frame2); + + ASSERT_EQ(generator.GetLayerId(encoded_image1), + generator.GetLayerId(encoded_image2)); + generator.SetHaltonSequenceIndex(0b11'1111'1111'1111, + generator.GetLayerId(encoded_image1)); + std::optional< + absl::variant> + data1 = generator.OnEncodedImage(encoded_image1); + std::optional< + absl::variant> + data2 = generator.OnEncodedImage(encoded_image2); + + ASSERT_TRUE(data1.has_value()); + ASSERT_TRUE(data2.has_value()); + ASSERT_TRUE(absl::holds_alternative(*data1)); + ASSERT_TRUE(absl::holds_alternative(*data2)); + + FrameInstrumentationData frame_instrumentation_data1 = + absl::get(*data1); + FrameInstrumentationData frame_instrumentation_data2 = + absl::get(*data2); + + EXPECT_EQ(frame_instrumentation_data1.sequence_index, 0); + EXPECT_EQ(frame_instrumentation_data2.sequence_index, 0b1000'0000); + + EXPECT_THAT(frame_instrumentation_data1.sample_values, + ElementsAre(1, 11, 19, 13, 3, 21, 6, 16, 18, 9, 7, 23, 1)); + EXPECT_THAT(frame_instrumentation_data2.sample_values, + ElementsAre(17, 10, 8, 24, 2, 12, 20, 13, 3, 21, 5, 15, 17)); +} + +TEST(FrameInstrumentationGeneratorTest, + SequenceIndexIncreasesCorrectlyAtNewKeyFrameAlreadyZeroes) { + FrameInstrumentationGenerator generator(VideoCodecType::kVideoCodecVP8); + VideoFrame frame1 = + VideoFrame::Builder() + .set_video_frame_buffer(MakeI420FrameBufferWithDifferentPixelValues()) + .set_rtp_timestamp(1) + .build(); + VideoFrame frame2 = + VideoFrame::Builder() + .set_video_frame_buffer(MakeI420FrameBufferWithDifferentPixelValues()) + .set_rtp_timestamp(2) + .build(); + EncodedImage encoded_image1; + encoded_image1.SetRtpTimestamp(1); + encoded_image1.SetFrameType(VideoFrameType::kVideoFrameKey); + encoded_image1.qp_ = 10; + encoded_image1._encodedWidth = kDefaultScaledWidth; + encoded_image1._encodedHeight = kDefaultScaledHeight; + + // Delta frame that is an upper layer of an SVC key frame. + EncodedImage encoded_image2; + encoded_image2.SetRtpTimestamp(2); + encoded_image2.SetFrameType(VideoFrameType::kVideoFrameKey); + encoded_image2.qp_ = 10; + encoded_image2._encodedWidth = kDefaultScaledWidth; + encoded_image2._encodedHeight = kDefaultScaledHeight; + + generator.OnCapturedFrame(frame1); + generator.OnCapturedFrame(frame2); + + ASSERT_EQ(generator.GetLayerId(encoded_image1), + generator.GetLayerId(encoded_image2)); + generator.SetHaltonSequenceIndex(0b1000'0000, + generator.GetLayerId(encoded_image1)); + + std::optional< + absl::variant> + data1 = generator.OnEncodedImage(encoded_image1); + std::optional< + absl::variant> + data2 = generator.OnEncodedImage(encoded_image2); + + ASSERT_TRUE(data1.has_value()); + ASSERT_TRUE(data2.has_value()); + ASSERT_TRUE(absl::holds_alternative(*data1)); + ASSERT_TRUE(absl::holds_alternative(*data2)); + + FrameInstrumentationData frame_instrumentation_data1 = + absl::get(*data1); + FrameInstrumentationData frame_instrumentation_data2 = + absl::get(*data2); + + EXPECT_EQ(frame_instrumentation_data1.sequence_index, 0b0000'1000'0000); + EXPECT_EQ(frame_instrumentation_data2.sequence_index, 0b0001'0000'0000); +} + +TEST(FrameInstrumentationGeneratorTest, GetterAndSetterOperatesAsExpected) { + FrameInstrumentationGenerator generator(VideoCodecType::kVideoCodecVP8); + // `std::nullopt` when uninitialized. + EXPECT_FALSE(generator.GetHaltonSequenceIndex(1).has_value()); + + // Zero is a valid index. + generator.SetHaltonSequenceIndex(0, 1); + std::optional index = generator.GetHaltonSequenceIndex(1); + EXPECT_TRUE(index.has_value()); + EXPECT_EQ(*index, 0); + +#if GTEST_HAS_DEATH_TEST + // Negative values are not allowed to be set. + EXPECT_DEATH(generator.SetHaltonSequenceIndex(-2, 1), + "Index must be non-negative"); + index = generator.GetHaltonSequenceIndex(1); + EXPECT_TRUE(index.has_value()); + EXPECT_EQ(*index, 0); + + // Values requiring more than 15 bits are not allowed. + EXPECT_DEATH(generator.SetHaltonSequenceIndex(0x4000, 1), + "Index must not be larger than 0x3FFF"); + index = generator.GetHaltonSequenceIndex(1); + EXPECT_TRUE(index.has_value()); + EXPECT_EQ(*index, 0); +#endif // GTEST_HAS_DEATH_TEST +} + } // namespace } // namespace webrtc diff --git a/video/corruption_detection/halton_sequence.cc b/video/corruption_detection/halton_sequence.cc index e977e903bc..c41647340a 100644 --- a/video/corruption_detection/halton_sequence.cc +++ b/video/corruption_detection/halton_sequence.cc @@ -59,6 +59,13 @@ std::vector HaltonSequence::GetNext() { return point; } +void HaltonSequence::SetCurrentIndex(int idx) { + if (idx >= 0) { + current_idx_ = idx; + } + RTC_DCHECK_GE(idx, 0) << "Index must be non-negative"; +} + void HaltonSequence::Reset() { HaltonSequence::current_idx_ = 0; } diff --git a/video/corruption_detection/halton_sequence.h b/video/corruption_detection/halton_sequence.h index fea7971cb2..576f32bdfb 100644 --- a/video/corruption_detection/halton_sequence.h +++ b/video/corruption_detection/halton_sequence.h @@ -35,7 +35,7 @@ class HaltonSequence { // interval [0,1). std::vector GetNext(); int GetCurrentIndex() const { return current_idx_; } - void SetCurrentIndex(int idx) { current_idx_ = idx; } + void SetCurrentIndex(int idx); void Reset(); private: