admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Each spatial layer can only have 1 QP value."

Browse Source 
This reverts commit 962b3935e44053641764beb0bd095540fe8cbd64.

Reason for revert: Breaks downstream tests.

Original change's description:
> 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}

Bug: webrtc:357636606
Change-Id: I60a2d4e1285f961f2ed2ea4c1d2e5942ea68b365
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/358721
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42737}
This commit is contained in:
Mirko Bonadei 2024-08-07 08:41:22 +00:00 committed by WebRTC LUCI CQ
parent 962b3935e4
commit b1b6129944
4 changed files with 40 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
View File

@ -365,8 +365,7 @@ 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.
// In singlecast, spatial/simulcast is expected not to be set.
int stream_index = encoded_image.SpatialIndex().value_or(
size_t 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,

12
test/pc/e2e/analyzer/video/default_video_quality_analyzer_frames_comparator.cc
View File

@ -501,14 +501,12 @@ 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_value] :
for (const auto& [spatial_layer, qp_values] :
frame_stats.spatial_layers_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));
for (SamplesStatsCounter::StatsSample qp : qp_values.GetTimedSamples()) {
qp.metadata = metadata;
stats->spatial_layers_qp[spatial_layer].AddSample(std::move(qp));
}
}
// Stats sliced on encoded frame type.

53
test/pc/e2e/analyzer/video/default_video_quality_analyzer_frames_comparator_test.cc
View File

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

9
test/pc/e2e/analyzer/video/default_video_quality_analyzer_internal_shared_objects.h
View File

@ -69,13 +69,8 @@ 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 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`.
// Sender side qp values per spatial layer. In case when spatial layer is not
// set for `webrtc::EncodedImage`, 0 is used as default.
std::map<int, SamplesStatsCounter> spatial_layers_qp;
absl::optional<int> decoded_frame_width = absl::nullopt;