Each spatial layer can only have 1 QP value.

As the code is now, it looks like it accepts that a spatial layer can have more than 1 QP value. These QP values according to the code are summed. However, to my best knowledge this cannot be the case and makes the code hard to read. Therefore, updating it with a check such that it checks that each spatial layer only have 1 QP value and would be easier for a future code reader.

Bug: webrtc:357636606
Change-Id: I650cac724811a1ddc7ab8933c1e1ac5fe844b61c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/358101
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Commit-Queue: Emil Vardar (xWF) <vardar@google.com>
Cr-Commit-Position: refs/heads/main@{#42736}
This commit is contained in:
Emil Vardar 2024-08-06 14:36:54 +00:00 committed by WebRTC LUCI CQ
parent a8dd3a36fa
commit 962b3935e4
4 changed files with 37 additions and 40 deletions

View File

@ -365,7 +365,8 @@ void DefaultVideoQualityAnalyzer::OnFrameEncoded(
// TODO(https://crbug.com/webrtc/14891): If we want to support a mix of
// simulcast and SVC we'll also need to consider the case where we have both
// simulcast and spatial indices.
size_t stream_index = encoded_image.SpatialIndex().value_or(
// In singlecast, spatial/simulcast is expected not to be set.
int stream_index = encoded_image.SpatialIndex().value_or(
encoded_image.SimulcastIndex().value_or(0));
frame_in_flight.OnFrameEncoded(
now, time_between_encoded_frames, encoded_image._frameType,

View File

@ -501,12 +501,14 @@ void DefaultVideoQualityAnalyzerFramesComparator::ProcessComparison(
frame_stats.encoded_image_size.bytes();
stats->target_encode_bitrate.AddSample(StatsSample(
frame_stats.target_encode_bitrate, frame_stats.encoded_time, metadata));
for (const auto& [spatial_layer, qp_values] :
for (const auto& [spatial_layer, qp_value] :
frame_stats.spatial_layers_qp) {
for (SamplesStatsCounter::StatsSample qp : qp_values.GetTimedSamples()) {
qp.metadata = metadata;
stats->spatial_layers_qp[spatial_layer].AddSample(std::move(qp));
}
// Each spatial/simulcast layer can only have one qp value.
RTC_CHECK_EQ(qp_value.GetTimedSamples().size(), 1)
<< "Can only be 1 QP value per spatial layer.";
SamplesStatsCounter::StatsSample qp = qp_value.GetTimedSamples()[0];
qp.metadata = metadata;
stats->spatial_layers_qp[spatial_layer].AddSample(std::move(qp));
}
// Stats sliced on encoded frame type.

View File

@ -503,8 +503,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
frame_stats.target_encode_bitrate = 2000;
frame_stats.spatial_layers_qp = {
{0, StatsCounter(
/*samples=*/{{5, Timestamp::Seconds(1)},
{5, Timestamp::Seconds(2)}})}};
/*samples=*/{{5, Timestamp::Seconds(1)}})}};
comparator.Start(/*max_threads_count=*/1);
comparator.EnsureStatsForStream(stream, sender, /*peers_count=*/2,
@ -534,7 +533,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
ExpectSizeAndAllElementsAre(stats.target_encode_bitrate, /*size=*/1,
/*value=*/2000.0);
EXPECT_THAT(stats.spatial_layers_qp, SizeIs(1));
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/2,
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/1,
/*value=*/5.0);
ExpectEmpty(stats.recv_key_frame_size_bytes);
ExpectEmpty(stats.recv_delta_frame_size_bytes);
@ -579,8 +578,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
frame_stats.target_encode_bitrate = 2000;
frame_stats.spatial_layers_qp = {
{0, StatsCounter(
/*samples=*/{{5, Timestamp::Seconds(1)},
{5, Timestamp::Seconds(2)}})}};
/*samples=*/{{5, Timestamp::Seconds(1)}})}};
comparator.Start(/*max_threads_count=*/1);
comparator.EnsureStatsForStream(stream, sender, /*peers_count=*/2,
@ -610,7 +608,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
ExpectSizeAndAllElementsAre(stats.target_encode_bitrate, /*size=*/1,
/*value=*/2000.0);
EXPECT_THAT(stats.spatial_layers_qp, SizeIs(1));
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/2,
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/1,
/*value=*/5.0);
ExpectEmpty(stats.recv_key_frame_size_bytes);
ExpectEmpty(stats.recv_delta_frame_size_bytes);
@ -655,8 +653,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
frame_stats.target_encode_bitrate = 2000;
frame_stats.spatial_layers_qp = {
{0, StatsCounter(
/*samples=*/{{5, Timestamp::Seconds(1)},
{5, Timestamp::Seconds(2)}})}};
/*samples=*/{{5, Timestamp::Seconds(1)}})}};
// Frame pre decoded
frame_stats.pre_decoded_frame_type = VideoFrameType::kVideoFrameKey;
frame_stats.pre_decoded_image_size = DataSize::Bytes(500);
@ -692,7 +689,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
ExpectSizeAndAllElementsAre(stats.target_encode_bitrate, /*size=*/1,
/*value=*/2000.0);
EXPECT_THAT(stats.spatial_layers_qp, SizeIs(1));
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/2,
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/1,
/*value=*/5.0);
ExpectSizeAndAllElementsAre(stats.recv_key_frame_size_bytes, /*size=*/1,
/*value=*/500.0);
@ -738,8 +735,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
frame_stats.target_encode_bitrate = 2000;
frame_stats.spatial_layers_qp = {
{0, StatsCounter(
/*samples=*/{{5, Timestamp::Seconds(1)},
{5, Timestamp::Seconds(2)}})}};
/*samples=*/{{5, Timestamp::Seconds(1)}})}};
// Frame pre decoded
frame_stats.pre_decoded_frame_type = VideoFrameType::kVideoFrameKey;
frame_stats.pre_decoded_image_size = DataSize::Bytes(500);
@ -782,7 +778,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
ExpectSizeAndAllElementsAre(stats.target_encode_bitrate, /*size=*/1,
/*value=*/2000.0);
EXPECT_THAT(stats.spatial_layers_qp, SizeIs(1));
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/2,
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/1,
/*value=*/5.0);
ExpectSizeAndAllElementsAre(stats.recv_key_frame_size_bytes, /*size=*/1,
/*value=*/500.0);
@ -829,8 +825,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
frame_stats.target_encode_bitrate = 2000;
frame_stats.spatial_layers_qp = {
{0, StatsCounter(
/*samples=*/{{5, Timestamp::Seconds(1)},
{5, Timestamp::Seconds(2)}})}};
/*samples=*/{{5, Timestamp::Seconds(1)}})}};
// Frame pre decoded
frame_stats.pre_decoded_frame_type = VideoFrameType::kVideoFrameKey;
frame_stats.pre_decoded_image_size = DataSize::Bytes(500);
@ -870,7 +865,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
ExpectSizeAndAllElementsAre(stats.target_encode_bitrate, /*size=*/1,
/*value=*/2000.0);
EXPECT_THAT(stats.spatial_layers_qp, SizeIs(1));
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/2,
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/1,
/*value=*/5.0);
ExpectSizeAndAllElementsAre(stats.recv_key_frame_size_bytes, /*size=*/1,
/*value=*/500.0);
@ -1038,8 +1033,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
frame_stats.target_encode_bitrate = 2000;
frame_stats.spatial_layers_qp = {
{0, StatsCounter(
/*samples=*/{{5, Timestamp::Seconds(1)},
{5, Timestamp::Seconds(2)}})}};
/*samples=*/{{5, Timestamp::Seconds(1)}})}};
comparator.Start(/*max_threads_count=*/1);
comparator.EnsureStatsForStream(stream, sender, /*peers_count=*/2,
@ -1069,7 +1063,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
ExpectSizeAndAllElementsAre(stats.target_encode_bitrate, /*size=*/1,
/*value=*/2000.0);
EXPECT_THAT(stats.spatial_layers_qp, SizeIs(1));
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/2,
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/1,
/*value=*/5.0);
ExpectEmpty(stats.recv_key_frame_size_bytes);
ExpectEmpty(stats.recv_delta_frame_size_bytes);
@ -1114,8 +1108,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
frame_stats.target_encode_bitrate = 2000;
frame_stats.spatial_layers_qp = {
{0, StatsCounter(
/*samples=*/{{5, Timestamp::Seconds(1)},
{5, Timestamp::Seconds(2)}})}};
/*samples=*/{{5, Timestamp::Seconds(1)}})}};
comparator.Start(/*max_threads_count=*/1);
comparator.EnsureStatsForStream(stream, sender, /*peers_count=*/2,
@ -1145,7 +1138,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
ExpectSizeAndAllElementsAre(stats.target_encode_bitrate, /*size=*/1,
/*value=*/2000.0);
EXPECT_THAT(stats.spatial_layers_qp, SizeIs(1));
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/2,
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/1,
/*value=*/5.0);
ExpectEmpty(stats.recv_key_frame_size_bytes);
ExpectEmpty(stats.recv_delta_frame_size_bytes);
@ -1267,8 +1260,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
frame_stats.target_encode_bitrate = 2000;
frame_stats.spatial_layers_qp = {
{0, StatsCounter(
/*samples=*/{{5, Timestamp::Seconds(1)},
{5, Timestamp::Seconds(2)}})}};
/*samples=*/{{5, Timestamp::Seconds(1)}})}};
// Frame pre decoded
frame_stats.pre_decoded_frame_type = VideoFrameType::kVideoFrameKey;
frame_stats.pre_decoded_image_size = DataSize::Bytes(500);
@ -1309,7 +1301,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
ExpectSizeAndAllElementsAre(stats.target_encode_bitrate, /*size=*/1,
/*value=*/2000.0);
EXPECT_THAT(stats.spatial_layers_qp, SizeIs(1));
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/2,
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/1,
/*value=*/5.0);
ExpectEmpty(stats.recv_key_frame_size_bytes);
ExpectEmpty(stats.recv_delta_frame_size_bytes);
@ -1355,8 +1347,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
frame_stats.target_encode_bitrate = 2000;
frame_stats.spatial_layers_qp = {
{0, StatsCounter(
/*samples=*/{{5, Timestamp::Seconds(1)},
{5, Timestamp::Seconds(2)}})}};
/*samples=*/{{5, Timestamp::Seconds(1)}})}};
// Frame pre decoded
frame_stats.pre_decoded_frame_type = VideoFrameType::kVideoFrameKey;
frame_stats.pre_decoded_image_size = DataSize::Bytes(500);
@ -1396,7 +1387,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
ExpectSizeAndAllElementsAre(stats.target_encode_bitrate, /*size=*/1,
/*value=*/2000.0);
EXPECT_THAT(stats.spatial_layers_qp, SizeIs(1));
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/2,
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/1,
/*value=*/5.0);
ExpectSizeAndAllElementsAre(stats.recv_key_frame_size_bytes, /*size=*/1,
/*value=*/500.0);
@ -1447,8 +1438,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
frame_stats.target_encode_bitrate = 2000;
frame_stats.spatial_layers_qp = {
{0, StatsCounter(
/*samples=*/{{5, Timestamp::Seconds(1)},
{5, Timestamp::Seconds(2)}})}};
/*samples=*/{{5, Timestamp::Seconds(1)}})}};
// Frame pre decoded
frame_stats.pre_decoded_frame_type = VideoFrameType::kVideoFrameKey;
frame_stats.pre_decoded_image_size = DataSize::Bytes(500);
@ -1492,7 +1482,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
ExpectSizeAndAllElementsAre(stats.target_encode_bitrate, /*size=*/1,
/*value=*/2000.0);
EXPECT_THAT(stats.spatial_layers_qp, SizeIs(1));
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/2,
ExpectSizeAndAllElementsAre(stats.spatial_layers_qp[0], /*size=*/1,
/*value=*/5.0);
ExpectSizeAndAllElementsAre(stats.recv_key_frame_size_bytes, /*size=*/1,
/*value=*/500.0);
@ -1540,8 +1530,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest, AllStatsHaveMetadataSet) {
frame_stats.target_encode_bitrate = 2000;
frame_stats.spatial_layers_qp = {
{0, StatsCounter(
/*samples=*/{{5, Timestamp::Seconds(1)},
{5, Timestamp::Seconds(2)}})}};
/*samples=*/{{5, Timestamp::Seconds(1)}})}};
// Frame pre decoded
frame_stats.pre_decoded_frame_type = VideoFrameType::kVideoFrameKey;
frame_stats.pre_decoded_image_size = DataSize::Bytes(500);

View File

@ -69,8 +69,13 @@ struct FrameStats {
VideoFrameType pre_decoded_frame_type = VideoFrameType::kEmptyFrame;
DataSize pre_decoded_image_size = DataSize::Bytes(0);
uint32_t target_encode_bitrate = 0;
// Sender side qp values per spatial layer. In case when spatial layer is not
// set for `webrtc::EncodedImage`, 0 is used as default.
// Sender side qp value per spatial layer. In case when spatial layer is not
// set for `webrtc::EncodedImage`, 0 is used as default. Each spatial layer
// can only have 1 QP value. Reason for using `SamplesStatsCounter` is because
// `SamplesStatsCounter::StatsSample` not having a default ctor and
// `Timestamp` is of interest.
// TODO(webrtc:357636606): Replace `SamplesStatsCounter` with optional<int>
// and `Timestamp`.
std::map<int, SamplesStatsCounter> spatial_layers_qp;
absl::optional<int> decoded_frame_width = absl::nullopt;