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

Make SimulcastIndex() and SpatialIndex() distinct (remove fallback).

Browse Source 
This CL removes the fallback logic to return the other index when the
one requested has not been set. This means we can remove the codec gates
that was previously needed because SpatialIndex() had multiple meanings,
resolving the TODOs previously added in
https://webrtc-review.googlesource.com/c/src/+/293343.

We have already migrated all known external dependencies from
SpatialIndex() to SimulcastIndex() where necessary, unblocking this CL.

PSA: https://groups.google.com/g/discuss-webrtc/c/SDAVg6xJ3gY

Bug: webrtc:14884
Change-Id: I82787505ab10be151e5f64965b270c45465d63a9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/293740
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39343}
This commit is contained in:
Henrik Boström 2023-02-16 09:57:05 +01:00 committed by WebRTC LUCI CQ
parent dc8a49abd8
commit 8ad4924936
5 changed files with 7 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
api/video/encoded_image.h
View File

@ -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<int> 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<int> SimulcastIndex() const { return simulcast_index_; }
void SetSimulcastIndex(absl::optional<int> 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<int> 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<int> SpatialIndex() const { return spatial_index_; }
void SetSpatialIndex(absl::optional<int> spatial_index) {
RTC_DCHECK_GE(spatial_index.value_or(0), 0);
RTC_DCHECK_LT(spatial_index.value_or(0), kMaxSpatialLayers);

9
call/rtp_payload_params.cc
View File

@ -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_;

13
call/rtp_video_sender.cc
View File

@ -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 =

13
video/send_statistics_proxy.cc
View File

@ -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.

4
video/video_stream_encoder_unittest.cc
View File

@ -9428,10 +9428,10 @@ TEST(VideoStreamEncoderFrameCadenceTest, UpdatesQualityConvergence) {
EXPECT_CALL(factory.GetMockFakeEncoder(), EncodeHook)
.WillRepeatedly(Invoke([](EncodedImage& encoded_image,
rtc::scoped_refptr<EncodedImageBuffer> 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;