diff --git a/api/video/encoded_image.h b/api/video/encoded_image.h index 5fdd2c666c..9046167470 100644 --- a/api/video/encoded_image.h +++ b/api/video/encoded_image.h @@ -91,15 +91,7 @@ class RTC_EXPORT EncodedImage { // Every simulcast layer (= encoding) has its own encoder and RTP stream. // There can be no dependencies between different simulcast layers. - absl::optional SimulcastIndex() const { - // Historically, SpatialIndex() has been used as both simulcast and spatial - // index (one or the other depending on codec). As to not break old code - // which doesn't call SetSimulcastIndex(), SpatialLayer() is used when the - // simulcast index is missing. - // TODO(https://crbug.com/webrtc/14884): When old code has been updated, - // never return `spatial_index_` here. - return simulcast_index_.has_value() ? simulcast_index_ : spatial_index_; - } + absl::optional SimulcastIndex() const { return simulcast_index_; } void SetSimulcastIndex(absl::optional simulcast_index) { RTC_DCHECK_GE(simulcast_index.value_or(0), 0); RTC_DCHECK_LT(simulcast_index.value_or(0), kMaxSimulcastStreams); @@ -109,15 +101,7 @@ class RTC_EXPORT EncodedImage { // Encoded images can have dependencies between spatial and/or temporal // layers, depending on the scalability mode used by the encoder. See diagrams // at https://w3c.github.io/webrtc-svc/#dependencydiagrams*. - absl::optional SpatialIndex() const { - // Historically, SpatialIndex() has been used as both simulcast and spatial - // index (one or the other depending on codec). As to not break old code - // that still uses the SpatialIndex() getter instead of SimulcastIndex() - // we fall back to `simulcast_index_` if `spatial_index_` is not set. - // TODO(https://crbug.com/webrtc/14884): When old code has been updated, - // never return `simulcast_index_` here. - return spatial_index_.has_value() ? spatial_index_ : simulcast_index_; - } + absl::optional SpatialIndex() const { return spatial_index_; } void SetSpatialIndex(absl::optional spatial_index) { RTC_DCHECK_GE(spatial_index.value_or(0), 0); RTC_DCHECK_LT(spatial_index.value_or(0), kMaxSpatialLayers); diff --git a/call/rtp_payload_params.cc b/call/rtp_payload_params.cc index b692abd976..18e6d9136b 100644 --- a/call/rtp_payload_params.cc +++ b/call/rtp_payload_params.cc @@ -202,15 +202,8 @@ RTPVideoHeader RtpPayloadParams::GetRtpVideoHeader( if (codec_specific_info) { PopulateRtpWithCodecSpecifics(*codec_specific_info, image.SpatialIndex(), &rtp_video_header); - // Currently, SimulcastIndex() could return the SpatialIndex() if not set - // correctly so gate on codec type. - // TODO(https://crbug.com/webrtc/14884): Delete this gating logic when - // SimulcastIndex() is guaranteed to be the stream index. - if (codec_specific_info->codecType != kVideoCodecVP9 && - codec_specific_info->codecType != kVideoCodecAV1) { - rtp_video_header.simulcastIdx = image.SimulcastIndex().value_or(0); - } } + rtp_video_header.simulcastIdx = image.SimulcastIndex().value_or(0); rtp_video_header.frame_type = image._frameType; rtp_video_header.rotation = image.rotation_; rtp_video_header.content_type = image.content_type_; diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 52714a55a5..98739a595e 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -571,18 +571,7 @@ EncodedImageCallback::Result RtpVideoSender::OnEncodedImage( return Result(Result::ERROR_SEND_FAILED); shared_frame_id_++; - size_t simulcast_index = 0; - // Currently, SimulcastIndex() could return the SpatialIndex() if not set - // correctly so gate on codec type. - // TODO(https://crbug.com/webrtc/14884): Delete this gating logic when - // SimulcastIndex() is guaranteed to be the stream index. - if (codec_specific_info && - (codec_specific_info->codecType == kVideoCodecVP8 || - codec_specific_info->codecType == kVideoCodecH264 || - codec_specific_info->codecType == kVideoCodecGeneric)) { - // Map spatial index to simulcast. - simulcast_index = encoded_image.SimulcastIndex().value_or(0); - } + size_t simulcast_index = encoded_image.SimulcastIndex().value_or(0); RTC_DCHECK_LT(simulcast_index, rtp_streams_.size()); uint32_t rtp_timestamp = diff --git a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc index 59144589fc..a5c9dd7884 100644 --- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc +++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc @@ -347,10 +347,15 @@ void DefaultVideoQualityAnalyzer::OnFrameEncoded( used_encoder.last_frame_id = frame_id; used_encoder.switched_on_at = now; used_encoder.switched_from_at = now; + // We could either have simulcast layers or spatial layers. + // 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( + encoded_image.SimulcastIndex().value_or(0)); frame_in_flight.OnFrameEncoded( now, encoded_image._frameType, DataSize::Bytes(encoded_image.size()), - stats.target_encode_bitrate, encoded_image.SpatialIndex().value_or(0), - stats.qp, used_encoder); + stats.target_encode_bitrate, stream_index, stats.qp, used_encoder); if (options_.report_infra_metrics) { analyzer_stats_.on_frame_encoded_processing_time_ms.AddSample( diff --git a/test/pc/e2e/analyzer/video/default_video_quality_analyzer_frame_in_flight.cc b/test/pc/e2e/analyzer/video/default_video_quality_analyzer_frame_in_flight.cc index df34dadaf0..fb87fd5f44 100644 --- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer_frame_in_flight.cc +++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer_frame_in_flight.cc @@ -88,14 +88,14 @@ void FrameInFlight::OnFrameEncoded(webrtc::Timestamp time, VideoFrameType frame_type, DataSize encoded_image_size, uint32_t target_encode_bitrate, - int spatial_layer, + int stream_index, int qp, StreamCodecInfo used_encoder) { encoded_time_ = time; frame_type_ = frame_type; encoded_image_size_ = encoded_image_size; target_encode_bitrate_ += target_encode_bitrate; - spatial_layers_qp_[spatial_layer].AddSample(SamplesStatsCounter::StatsSample{ + stream_layers_qp_[stream_index].AddSample(SamplesStatsCounter::StatsSample{ .value = static_cast(qp), .time = time}); // Update used encoder info. If simulcast/SVC is used, this method can // be called multiple times, in such case we should preserve the value @@ -186,7 +186,7 @@ FrameStats FrameInFlight::GetStatsForPeer(size_t peer) const { stats.encoded_frame_type = frame_type_; stats.encoded_image_size = encoded_image_size_; stats.used_encoder = used_encoder_; - stats.spatial_layers_qp = spatial_layers_qp_; + stats.spatial_layers_qp = stream_layers_qp_; absl::optional receiver_stats = MaybeGetValue(receiver_stats_, peer); diff --git a/test/pc/e2e/analyzer/video/default_video_quality_analyzer_frame_in_flight.h b/test/pc/e2e/analyzer/video/default_video_quality_analyzer_frame_in_flight.h index 52a526d09b..3c5bc95743 100644 --- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer_frame_in_flight.h +++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer_frame_in_flight.h @@ -88,7 +88,7 @@ class FrameInFlight { VideoFrameType frame_type, DataSize encoded_image_size, uint32_t target_encode_bitrate, - int spatial_layer, + int stream_index, int qp, StreamCodecInfo used_encoder); @@ -155,9 +155,9 @@ class FrameInFlight { VideoFrameType frame_type_ = VideoFrameType::kEmptyFrame; DataSize encoded_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. - std::map spatial_layers_qp_; + // Sender side qp values per spatial or simulcast layer. If neither the + // spatial or simulcast index is set in `webrtc::EncodedImage`, 0 is used. + std::map stream_layers_qp_; // Can be not set if frame was dropped by encoder. absl::optional used_encoder_ = absl::nullopt; // Map from the receiver peer's index to frame stats for that peer. diff --git a/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc b/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc index e814ba88b7..7a2b3165d6 100644 --- a/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc +++ b/test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.cc @@ -279,8 +279,14 @@ EncodedImageCallback::Result QualityAnalyzingVideoEncoder::OnEncodedImage( discard = ShouldDiscard(frame_id, encoded_image); if (!discard) { - target_encode_bitrate = bitrate_allocation_.GetSpatialLayerSum( - encoded_image.SpatialIndex().value_or(0)); + // We could either have simulcast layers or spatial layers. + // 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( + encoded_image.SimulcastIndex().value_or(0)); + target_encode_bitrate = + bitrate_allocation_.GetSpatialLayerSum(stream_index); } codec_name = std::string(CodecTypeToPayloadString(codec_settings_.codecType)) + "_" + @@ -326,7 +332,12 @@ bool QualityAnalyzingVideoEncoder::ShouldDiscard( if (!emulated_sfu_config) return false; - int cur_spatial_index = encoded_image.SpatialIndex().value_or(0); + // We could either have simulcast layers or spatial layers. + // 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. + int cur_stream_index = encoded_image.SpatialIndex().value_or( + encoded_image.SimulcastIndex().value_or(0)); int cur_temporal_index = encoded_image.TemporalIndex().value_or(0); if (emulated_sfu_config->target_temporal_index && @@ -338,12 +349,12 @@ bool QualityAnalyzingVideoEncoder::ShouldDiscard( case SimulcastMode::kSimulcast: // In simulcast mode only encoded images with required spatial index are // interested, so all others have to be discarded. - return cur_spatial_index != *emulated_sfu_config->target_layer_index; + return cur_stream_index != *emulated_sfu_config->target_layer_index; case SimulcastMode::kSVC: // In SVC mode encoded images with spatial indexes that are equal or // less than required one are interesting, so all above have to be // discarded. - return cur_spatial_index > *emulated_sfu_config->target_layer_index; + return cur_stream_index > *emulated_sfu_config->target_layer_index; case SimulcastMode::kKSVC: // In KSVC mode for key frame encoded images with spatial indexes that // are equal or less than required one are interesting, so all above @@ -353,8 +364,8 @@ bool QualityAnalyzingVideoEncoder::ShouldDiscard( // all of temporal layer 0 for now. if (encoded_image._frameType == VideoFrameType::kVideoFrameKey || cur_temporal_index == 0) - return cur_spatial_index > *emulated_sfu_config->target_layer_index; - return cur_spatial_index != *emulated_sfu_config->target_layer_index; + return cur_stream_index > *emulated_sfu_config->target_layer_index; + return cur_stream_index != *emulated_sfu_config->target_layer_index; case SimulcastMode::kNormal: RTC_DCHECK_NOTREACHED() << "Analyzing encoder is in kNormal mode, but " "target_layer_index is set"; diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc index a64ab221db..12e88ac16d 100644 --- a/video/send_statistics_proxy.cc +++ b/video/send_statistics_proxy.cc @@ -937,18 +937,7 @@ void SendStatisticsProxy::OnMinPixelLimitReached() { void SendStatisticsProxy::OnSendEncodedImage( const EncodedImage& encoded_image, const CodecSpecificInfo* codec_info) { - // Simulcast is used for VP8, H264 and Generic. - // Currently, SimulcastIndex() could return the SpatialIndex() if not set - // correctly so gate on codec type. - // TODO(https://crbug.com/webrtc/14884): Delete this gating logic when - // SimulcastIndex() is guaranteed to be the stream index. - int simulcast_idx = - (codec_info && (codec_info->codecType == kVideoCodecVP8 || - codec_info->codecType == kVideoCodecH264 || - codec_info->codecType == kVideoCodecGeneric)) - ? encoded_image.SimulcastIndex().value_or(0) - : 0; - + int simulcast_idx = encoded_image.SimulcastIndex().value_or(0); MutexLock lock(&mutex_); ++stats_.frames_encoded; // The current encode frame rate is based on previously encoded frames. diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index a903c2115f..583bb8ca0b 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -241,12 +241,7 @@ class QualityTestVideoEncoder : public VideoEncoder, Result OnEncodedImage(const EncodedImage& encoded_image, const CodecSpecificInfo* codec_specific_info) override { if (codec_specific_info) { - int simulcast_index; - if (codec_specific_info->codecType == kVideoCodecVP9) { - simulcast_index = 0; - } else { - simulcast_index = encoded_image.SpatialIndex().value_or(0); - } + int simulcast_index = encoded_image.SimulcastIndex().value_or(0); RTC_DCHECK_GE(simulcast_index, 0); if (analyzer_) { analyzer_->PostEncodeOnFrame(simulcast_index, diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 65aa0760c5..b0666a4744 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -2434,8 +2434,13 @@ void VideoStreamEncoder::RunPostEncode(const EncodedImage& encoded_image, stream_resource_manager_.OnEncodeCompleted(encoded_image, time_sent_us, encode_duration_us, frame_size); if (bitrate_adjuster_) { - bitrate_adjuster_->OnEncodedFrame( - frame_size, encoded_image.SpatialIndex().value_or(0), temporal_index); + // We could either have simulcast layers or spatial layers. + // 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. + int stream_index = encoded_image.SpatialIndex().value_or( + encoded_image.SimulcastIndex().value_or(0)); + bitrate_adjuster_->OnEncodedFrame(frame_size, stream_index, temporal_index); } } diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 908c96fc43..c451b6fe20 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -9473,10 +9473,10 @@ TEST(VideoStreamEncoderFrameCadenceTest, UpdatesQualityConvergence) { EXPECT_CALL(factory.GetMockFakeEncoder(), EncodeHook) .WillRepeatedly(Invoke([](EncodedImage& encoded_image, rtc::scoped_refptr buffer) { - // This sets spatial index 0 content to be at target quality, while + // This sets simulcast index 0 content to be at target quality, while // index 1 content is not. encoded_image.qp_ = kVp8SteadyStateQpThreshold + - (encoded_image.SpatialIndex() == 0 ? 0 : 1); + (encoded_image.SimulcastIndex() == 0 ? 0 : 1); CodecSpecificInfo codec_specific; codec_specific.codecType = kVideoCodecVP8; return codec_specific;