diff --git a/api/video/encoded_image.h b/api/video/encoded_image.h index 9046167470..5fdd2c666c 100644 --- a/api/video/encoded_image.h +++ b/api/video/encoded_image.h @@ -91,7 +91,15 @@ 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 { return simulcast_index_; } + 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_; + } void SetSimulcastIndex(absl::optional simulcast_index) { RTC_DCHECK_GE(simulcast_index.value_or(0), 0); RTC_DCHECK_LT(simulcast_index.value_or(0), kMaxSimulcastStreams); @@ -101,7 +109,15 @@ 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 { return spatial_index_; } + 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_; + } 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 18e6d9136b..b692abd976 100644 --- a/call/rtp_payload_params.cc +++ b/call/rtp_payload_params.cc @@ -202,8 +202,15 @@ 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 98739a595e..52714a55a5 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -571,7 +571,18 @@ EncodedImageCallback::Result RtpVideoSender::OnEncodedImage( return Result(Result::ERROR_SEND_FAILED); shared_frame_id_++; - size_t simulcast_index = encoded_image.SimulcastIndex().value_or(0); + 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); + } RTC_DCHECK_LT(simulcast_index, rtp_streams_.size()); uint32_t rtp_timestamp = diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc index 12e88ac16d..a64ab221db 100644 --- a/video/send_statistics_proxy.cc +++ b/video/send_statistics_proxy.cc @@ -937,7 +937,18 @@ void SendStatisticsProxy::OnMinPixelLimitReached() { void SendStatisticsProxy::OnSendEncodedImage( const EncodedImage& encoded_image, const CodecSpecificInfo* codec_info) { - int simulcast_idx = encoded_image.SimulcastIndex().value_or(0); + // 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; + MutexLock lock(&mutex_); ++stats_.frames_encoded; // The current encode frame rate is based on previously encoded frames. diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index f2963c6778..cdd4c75ab7 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -9428,10 +9428,10 @@ TEST(VideoStreamEncoderFrameCadenceTest, UpdatesQualityConvergence) { EXPECT_CALL(factory.GetMockFakeEncoder(), EncodeHook) .WillRepeatedly(Invoke([](EncodedImage& encoded_image, rtc::scoped_refptr buffer) { - // This sets simulcast index 0 content to be at target quality, while + // This sets spatial index 0 content to be at target quality, while // index 1 content is not. encoded_image.qp_ = kVp8SteadyStateQpThreshold + - (encoded_image.SimulcastIndex() == 0 ? 0 : 1); + (encoded_image.SpatialIndex() == 0 ? 0 : 1); CodecSpecificInfo codec_specific; codec_specific.codecType = kVideoCodecVP8; return codec_specific;