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

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

Browse Source 
This reverts commit 8ad4924936dea2bd97990b0a951df93f7526f0ff.

Reason for revert: Breaks downstream projects

Original change's description:
> Make SimulcastIndex() and SpatialIndex() distinct (remove fallback).
>
> 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}

Bug: webrtc:14884
Change-Id: Ibcb834a1519930336fa50e8e9d8d0137972e28e6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/294282
Owners-Override: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39347}
This commit is contained in:
Henrik Boström 2023-02-20 12:44:53 +00:00 committed by WebRTC LUCI CQ
parent e5ab6c3bb0
commit 79a6f87648
5 changed files with 52 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
api/video/encoded_image.h
View File

@ -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<int> SimulcastIndex() const { return simulcast_index_; }
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_;
}
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);
@ -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<int> SpatialIndex() const { return spatial_index_; }
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_;
}
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,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_;

13
call/rtp_video_sender.cc
View File

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

13
video/send_statistics_proxy.cc
View File

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

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