From faef5de87c311f9c2fbf4f5a336e8161bebf782e Mon Sep 17 00:00:00 2001 From: Qiu Jianlin Date: Fri, 1 Nov 2024 20:18:15 +0800 Subject: [PATCH] Cleanup H.265 TODOs. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cleanup some of the TODOs for H.265. They are either invalid or their handling should be merged with other codec types. Bug: chromium:41480904 Change-Id: I76263354b1b87035e240d77283b21a9a26dcb45b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366044 Reviewed-by: Henrik Boström Commit-Queue: Jianlin Qiu Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/main@{#43359} --- call/rtp_payload_params.cc | 13 ++++---- modules/rtp_rtcp/source/rtp_sender_video.cc | 14 ++++++--- modules/rtp_rtcp/source/rtp_video_header.cc | 7 ++--- .../video_coding/utility/simulcast_utility.cc | 7 +++-- test/testsupport/ivf_video_frame_generator.cc | 4 +-- video/video_stream_encoder.cc | 31 +++++++++++++------ 6 files changed, 48 insertions(+), 28 deletions(-) diff --git a/call/rtp_payload_params.cc b/call/rtp_payload_params.cc index 80e9f421c1..acc9e3ca18 100644 --- a/call/rtp_payload_params.cc +++ b/call/rtp_payload_params.cc @@ -111,11 +111,10 @@ void PopulateRtpWithCodecSpecifics(const CodecSpecificInfo& info, info.codecSpecific.H264.packetization_mode; return; } + // These codec types do not have codec-specifics. case kVideoCodecGeneric: - rtp->codec = kVideoCodecGeneric; - return; - // TODO(bugs.webrtc.org/13485): Implement H265 codec specific info - default: + case kVideoCodecH265: + case kVideoCodecAV1: return; } } @@ -353,7 +352,8 @@ void RtpPayloadParams::SetGeneric(const CodecSpecificInfo* codec_specific_info, } return; case VideoCodecType::kVideoCodecAV1: - // TODO(philipel): Implement AV1 to generic descriptor. + // Codec-specifics is not supported for AV1. We convert from the + // generic_frame_info. return; case VideoCodecType::kVideoCodecH264: if (codec_specific_info) { @@ -362,7 +362,8 @@ void RtpPayloadParams::SetGeneric(const CodecSpecificInfo* codec_specific_info, } return; case VideoCodecType::kVideoCodecH265: - // TODO(bugs.webrtc.org/13485): Implement H265 to generic descriptor. + // Codec-specifics is not supported for H.265. We convert from the + // generic_frame_info. return; } RTC_DCHECK_NOTREACHED() << "Unsupported codec."; diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 95c5c41bb3..8a21d0523f 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -102,6 +102,12 @@ bool MinimizeDescriptor(RTPVideoHeader* video_header) { } bool IsBaseLayer(const RTPVideoHeader& video_header) { + // For AV1 & H.265 we fetch temporal index from the generic descriptor. + if (video_header.generic) { + const auto& generic = video_header.generic.value(); + return (generic.temporal_index == 0 || + generic.temporal_index == kNoTemporalIdx); + } switch (video_header.codec) { case kVideoCodecVP8: { const auto& vp8 = @@ -117,11 +123,11 @@ bool IsBaseLayer(const RTPVideoHeader& video_header) { // TODO(kron): Implement logic for H264 once WebRTC supports temporal // layers for H264. break; + // These codecs do not have codec-specifics, from which we can fetch + // temporal index. case kVideoCodecH265: - // TODO(bugs.webrtc.org/13485): Implement logic for H265 once WebRTC - // supports temporal layers for H265. - break; - default: + case kVideoCodecAV1: + case kVideoCodecGeneric: break; } return true; diff --git a/modules/rtp_rtcp/source/rtp_video_header.cc b/modules/rtp_rtcp/source/rtp_video_header.cc index c37cc65df3..558b58289a 100644 --- a/modules/rtp_rtcp/source/rtp_video_header.cc +++ b/modules/rtp_rtcp/source/rtp_video_header.cc @@ -59,11 +59,10 @@ VideoFrameMetadata RTPVideoHeader::GetAsMetadata() const { metadata.SetRTPVideoHeaderCodecSpecifics( absl::get(video_type_header)); break; + // These codec types do not have codec-specifics. case VideoCodecType::kVideoCodecH265: - // TODO(bugs.webrtc.org/13485) - break; - default: - // Codec-specifics are not supported for this codec. + case VideoCodecType::kVideoCodecAV1: + case VideoCodecType::kVideoCodecGeneric: break; } return metadata; diff --git a/modules/video_coding/utility/simulcast_utility.cc b/modules/video_coding/utility/simulcast_utility.cc index b637e7fdd0..162f362dbe 100644 --- a/modules/video_coding/utility/simulcast_utility.cc +++ b/modules/video_coding/utility/simulcast_utility.cc @@ -105,10 +105,11 @@ int SimulcastUtility::NumberOfTemporalLayers(const VideoCodec& codec, case kVideoCodecH264: num_temporal_layers = codec.H264().numberOfTemporalLayers; break; + // For AV1 and H.265 we get temporal layer count from scalability mode, + // instead of from codec-specifics. + case kVideoCodecAV1: case kVideoCodecH265: - // TODO(bugs.webrtc.org/13485) - break; - default: + case kVideoCodecGeneric: break; } } diff --git a/test/testsupport/ivf_video_frame_generator.cc b/test/testsupport/ivf_video_frame_generator.cc index 3410205576..5f4906e600 100644 --- a/test/testsupport/ivf_video_frame_generator.cc +++ b/test/testsupport/ivf_video_frame_generator.cc @@ -43,9 +43,9 @@ std::unique_ptr CreateDecoder(const Environment& env, case VideoCodecType::kVideoCodecAV1: return CreateDav1dDecoder(); case VideoCodecType::kVideoCodecH265: - // TODO: bugs.webrtc.org/13485 - implement H265 decoder + // No H.265 SW decoder implementation will be provided. return nullptr; - default: + case VideoCodecType::kVideoCodecGeneric: return nullptr; } } diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 3d86a14123..e2037575fa 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -151,7 +151,7 @@ bool RequiresEncoderReset(const VideoCodec& prev_send_codec, } break; case kVideoCodecH265: - // TODO(bugs.webrtc.org/13485): Implement new send codec H265 + // No H.265 specific handling needed. [[fallthrough]]; default: break; @@ -1169,7 +1169,11 @@ void VideoStreamEncoder::ReconfigureEncoder() { env_.field_trials(), encoder_config_, streams); if (encoder_config_.codec_type == kVideoCodecVP9 || - encoder_config_.codec_type == kVideoCodecAV1) { + encoder_config_.codec_type == kVideoCodecAV1 +#ifdef RTC_ENABLE_H265 + || encoder_config_.codec_type == kVideoCodecH265 +#endif + ) { // Spatial layers configuration might impose some parity restrictions, // thus some cropping might be needed. RTC_CHECK_GE(last_frame_info_->width, codec.width); @@ -1205,7 +1209,11 @@ void VideoStreamEncoder::ReconfigureEncoder() { } } if (encoder_config_.codec_type == kVideoCodecVP9 || - encoder_config_.codec_type == kVideoCodecAV1) { + encoder_config_.codec_type == kVideoCodecAV1 +#ifdef RTC_ENABLE_H265 + || encoder_config_.codec_type == kVideoCodecH265 +#endif + ) { log_stream << ", spatial layers: "; for (int i = 0; i < GetNumSpatialLayers(codec); ++i) { log_stream << "{" << i << ": " << codec.spatialLayers[i].width << "x" @@ -1336,7 +1344,8 @@ void VideoStreamEncoder::ReconfigureEncoder() { num_layers = codec.VP8()->numberOfTemporalLayers; } else if (codec.codecType == kVideoCodecVP9) { num_layers = codec.VP9()->numberOfTemporalLayers; - } else if (codec.codecType == kVideoCodecAV1 && + } else if ((codec.codecType == kVideoCodecAV1 || + codec.codecType == kVideoCodecH265) && codec.GetScalabilityMode().has_value()) { num_layers = ScalabilityModeToNumTemporalLayers(*(codec.GetScalabilityMode())); @@ -1348,7 +1357,6 @@ void VideoStreamEncoder::ReconfigureEncoder() { // TODO(sprang): Add a better way to disable frame dropping. num_layers = codec.simulcastStream[0].numberOfTemporalLayers; } else { - // TODO(bugs.webrtc.org/13485): Implement H265 temporal layer num_layers = 1; } @@ -1392,10 +1400,15 @@ void VideoStreamEncoder::ReconfigureEncoder() { break; } } - // Set min_bitrate_bps, max_bitrate_bps, and max padding bit rate for VP9 - // and AV1 and leave only one stream containing all necessary information. - if ((encoder_config_.codec_type == kVideoCodecVP9 || - encoder_config_.codec_type == kVideoCodecAV1) && + // Set min_bitrate_bps, max_bitrate_bps, and max padding bit rate for VP9, + // AV1 and H.265, and leave only one stream containing all necessary + // information. + if (( +#ifdef RTC_ENABLE_H265 + encoder_config_.codec_type == kVideoCodecH265 || +#endif + encoder_config_.codec_type == kVideoCodecVP9 || + encoder_config_.codec_type == kVideoCodecAV1) && single_stream_or_non_first_inactive) { // Lower max bitrate to the level codec actually can produce. streams[0].max_bitrate_bps =