From c8325cbdd100edf721665c98f0f688e83f836d26 Mon Sep 17 00:00:00 2001 From: Dave Lane <42013603+ReenigneArcher@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:27:39 -0400 Subject: [PATCH] fix(amf): wire h264_amf coder and profile mapping (#5442) Co-authored-by: k4idyn <252943111+k4idyn@users.noreply.github.com> --- src/video.cpp | 76 +++++++++++++++++++++++---------------- src/video.h | 24 +++++++++++++ tests/unit/test_video.cpp | 45 +++++++++++++++++++++++ 3 files changed, 114 insertions(+), 31 deletions(-) diff --git a/src/video.cpp b/src/video.cpp index a6981a153..02f996df5 100644 --- a/src/video.cpp +++ b/src/video.cpp @@ -8,6 +8,7 @@ #include #include #include +#include // lib includes #include @@ -143,6 +144,18 @@ namespace video { } // namespace qsv + int select_h264_profile(std::string_view encoder_name, const config_t &config, int amd_coder) { + if (config.chromaSamplingType == 1) { + return AV_PROFILE_H264_HIGH_444_PREDICTIVE; + } + + if (encoder_name == "h264_amf"sv && amd_coder == std::to_underlying(amf::coder_e::cavlc)) { + return AV_PROFILE_H264_CONSTRAINED_BASELINE; + } + + return AV_PROFILE_H264_HIGH; + } + /** * @brief Create an FFmpeg hardware device buffer for D3D11VA input. * @@ -207,8 +220,8 @@ namespace video { // If we require aspect ratio padding, copy the output frame into the final padded frame if (requires_padding) { - auto fmt_desc = av_pix_fmt_desc_get((AVPixelFormat) sws_output_frame->format); - auto planes = av_pix_fmt_count_planes((AVPixelFormat) sws_output_frame->format); + auto fmt_desc = av_pix_fmt_desc_get(static_cast(sws_output_frame->format)); + auto planes = av_pix_fmt_count_planes(static_cast(sws_output_frame->format)); for (int plane = 0; plane < planes; plane++) { auto shift_h = plane == 0 ? 0 : fmt_desc->log2_chroma_h; auto shift_w = plane == 0 ? 0 : fmt_desc->log2_chroma_w; @@ -216,7 +229,7 @@ namespace video { // Copy line-by-line to preserve leading padding for each row for (int line = 0; line < sws_output_frame->height >> shift_h; line++) { - memcpy(sw_frame->data[plane] + offset + (line * sw_frame->linesize[plane]), sws_output_frame->data[plane] + (line * sws_output_frame->linesize[plane]), (size_t) (sws_output_frame->width >> shift_w) * fmt_desc->comp[plane].step); + memcpy(sw_frame->data[plane] + offset + (line * sw_frame->linesize[plane]), sws_output_frame->data[plane] + (line * sws_output_frame->linesize[plane]), static_cast(sws_output_frame->width >> shift_w) * fmt_desc->comp[plane].step); } } } @@ -275,7 +288,7 @@ namespace video { av_frame_get_buffer(frame, 0); av_frame_make_writable(frame); ptrdiff_t linesize[4] = {frame->linesize[0], frame->linesize[1], frame->linesize[2], frame->linesize[3]}; - av_image_fill_black(frame->data, linesize, (AVPixelFormat) frame->format, frame->color_range, frame->width, frame->height); + av_image_fill_black(frame->data, linesize, static_cast(frame->format), frame->color_range, frame->width, frame->height); } /** @@ -307,7 +320,7 @@ namespace video { auto out_height = frame->height; // Ensure aspect ratio is maintained - auto scalar = std::fminf((float) out_width / in_width, (float) out_height / in_height); + auto scalar = std::fminf(static_cast(out_width) / in_width, static_cast(out_height) / in_height); out_width = in_width * scalar; out_height = in_height * scalar; @@ -777,11 +790,11 @@ namespace video { }, { // SDR-specific options - {"profile"s, (int) nv::profile_hevc_e::main}, + {"profile"s, std::to_underlying(nv::profile_hevc_e::main)}, }, { // HDR-specific options - {"profile"s, (int) nv::profile_hevc_e::main_10}, + {"profile"s, std::to_underlying(nv::profile_hevc_e::main_10)}, }, {}, // YUV444 SDR-specific options {}, // YUV444 HDR-specific options @@ -804,7 +817,7 @@ namespace video { }, { // SDR-specific options - {"profile"s, (int) nv::profile_h264_e::high}, + {"profile"s, std::to_underlying(nv::profile_h264_e::high)}, }, {}, // HDR-specific options {}, // YUV444 SDR-specific options @@ -843,19 +856,19 @@ namespace video { }, { // SDR-specific options - {"profile"s, (int) qsv::profile_av1_e::main}, + {"profile"s, std::to_underlying(qsv::profile_av1_e::main)}, }, { // HDR-specific options - {"profile"s, (int) qsv::profile_av1_e::main}, + {"profile"s, std::to_underlying(qsv::profile_av1_e::main)}, }, { // YUV444 SDR-specific options - {"profile"s, (int) qsv::profile_av1_e::high}, + {"profile"s, std::to_underlying(qsv::profile_av1_e::high)}, }, { // YUV444 HDR-specific options - {"profile"s, (int) qsv::profile_av1_e::high}, + {"profile"s, std::to_underlying(qsv::profile_av1_e::high)}, }, {}, // Fallback options "av1_qsv"s, @@ -873,19 +886,19 @@ namespace video { }, { // SDR-specific options - {"profile"s, (int) qsv::profile_hevc_e::main}, + {"profile"s, std::to_underlying(qsv::profile_hevc_e::main)}, }, { // HDR-specific options - {"profile"s, (int) qsv::profile_hevc_e::main_10}, + {"profile"s, std::to_underlying(qsv::profile_hevc_e::main_10)}, }, { // YUV444 SDR-specific options - {"profile"s, (int) qsv::profile_hevc_e::rext}, + {"profile"s, std::to_underlying(qsv::profile_hevc_e::rext)}, }, { // YUV444 HDR-specific options - {"profile"s, (int) qsv::profile_hevc_e::rext}, + {"profile"s, std::to_underlying(qsv::profile_hevc_e::rext)}, }, { // Fallback options @@ -911,12 +924,12 @@ namespace video { }, { // SDR-specific options - {"profile"s, (int) qsv::profile_h264_e::high}, + {"profile"s, std::to_underlying(qsv::profile_h264_e::high)}, }, {}, // HDR-specific options { // YUV444 SDR-specific options - {"profile"s, (int) qsv::profile_h264_e::high_444p}, + {"profile"s, std::to_underlying(qsv::profile_h264_e::high_444p)}, }, {}, // YUV444 HDR-specific options { @@ -1022,6 +1035,7 @@ namespace video { {"rc"s, &config::video.amd.amd_rc_h264}, {"usage"s, &config::video.amd.amd_usage_h264}, {"vbaq"s, &config::video.amd.amd_vbaq}, + {"coder"s, &config::video.amd.amd_coder}, {"enforce_hrd"s, &config::video.amd.amd_enforce_hrd}, }, {}, // SDR-specific options @@ -1719,7 +1733,7 @@ namespace video { // Process any pending display switch with the new list of displays if (switch_display_event->peek()) { - display_p = std::clamp(*switch_display_event->pop(), 0, (int) display_names.size() - 1); + display_p = std::clamp(*switch_display_event->pop(), 0, static_cast(display_names.size()) - 1); } // reset_display() will sleep between retries @@ -1743,7 +1757,7 @@ namespace video { case platf::capture_e::interrupted: return; default: - BOOST_LOG(error) << "Unrecognized capture status ["sv << (int) status << ']'; + BOOST_LOG(error) << "Unrecognized capture status ["sv << std::to_underlying(status) << ']'; return; } } @@ -1808,16 +1822,16 @@ namespace video { vps = std::move(hevc.vps); session.replacements.emplace_back( - std::string_view((char *) std::begin(vps.old), vps.old.size()), - std::string_view((char *) std::begin(vps._new), vps._new.size()) + std::string_view(reinterpret_cast(std::begin(vps.old)), vps.old.size()), + std::string_view(reinterpret_cast(std::begin(vps._new)), vps._new.size()) ); } session.inject = 0; session.replacements.emplace_back( - std::string_view((char *) std::begin(sps.old), sps.old.size()), - std::string_view((char *) std::begin(sps._new), sps._new.size()) + std::string_view(reinterpret_cast(std::begin(sps.old)), sps.old.size()), + std::string_view(reinterpret_cast(std::begin(sps._new)), sps._new.size()) ); } @@ -1965,7 +1979,7 @@ namespace video { case 0: // 10-bit h264 encoding is not supported by our streaming protocol assert(!config.dynamicRange); - ctx->profile = (config.chromaSamplingType == 1) ? AV_PROFILE_H264_HIGH_444_PREDICTIVE : AV_PROFILE_H264_HIGH; + ctx->profile = select_h264_profile(video_format.name, config, config::video.amd.amd_coder); break; case 1: @@ -2276,7 +2290,7 @@ namespace video { std::move(encode_device_final), // 0 ==> don't inject, 1 ==> inject for h264, 2 ==> inject for hevc - config.videoFormat <= 1 ? (1 - (int) video_format[encoder_t::VUI_PARAMETERS]) * (1 + config.videoFormat) : 0 + config.videoFormat <= 1 ? (1 - static_cast(video_format[encoder_t::VUI_PARAMETERS])) * (1 + config.videoFormat) : 0 ); return session; @@ -2643,7 +2657,7 @@ namespace video { // Process any pending display switch with the new list of displays if (switch_display_event->peek()) { - display_p = std::clamp(*switch_display_event->pop(), 0, (int) display_names.size() - 1); + display_p = std::clamp(*switch_display_event->pop(), 0, static_cast(display_names.size()) - 1); } // reset_display() will sleep between retries @@ -3381,7 +3395,7 @@ namespace video { BOOST_LOG(debug) << "------ h264 ------"sv; for (int x = 0; x < encoder_t::MAX_FLAGS; ++x) { - auto flag = (encoder_t::flag_e) x; + auto flag = static_cast(x); BOOST_LOG(debug) << encoder_t::from_flag(flag) << (encoder.h264[flag] ? ": supported"sv : ": unsupported"sv); } BOOST_LOG(debug) << "-------------------"sv; @@ -3390,7 +3404,7 @@ namespace video { if (encoder.hevc[encoder_t::PASSED]) { BOOST_LOG(debug) << "------ hevc ------"sv; for (int x = 0; x < encoder_t::MAX_FLAGS; ++x) { - auto flag = (encoder_t::flag_e) x; + auto flag = static_cast(x); BOOST_LOG(debug) << encoder_t::from_flag(flag) << (encoder.hevc[flag] ? ": supported"sv : ": unsupported"sv); } BOOST_LOG(debug) << "-------------------"sv; @@ -3401,7 +3415,7 @@ namespace video { if (encoder.av1[encoder_t::PASSED]) { BOOST_LOG(debug) << "------ av1 ------"sv; for (int x = 0; x < encoder_t::MAX_FLAGS; ++x) { - auto flag = (encoder_t::flag_e) x; + auto flag = static_cast(x); BOOST_LOG(debug) << encoder_t::from_flag(flag) << (encoder.av1[flag] ? ": supported"sv : ": unsupported"sv); } BOOST_LOG(debug) << "-------------------"sv; @@ -3566,7 +3580,7 @@ namespace video { std::fill_n((std::uint8_t *) ctx, sizeof(AVD3D11VADeviceContext), 0); - auto device = (ID3D11Device *) encode_device->data; + auto device = static_cast(encode_device->data); device->AddRef(); ctx->device = device; diff --git a/src/video.h b/src/video.h index 26f2e8118..b70ed53b3 100644 --- a/src/video.h +++ b/src/video.h @@ -6,6 +6,7 @@ // standard includes #include +#include // local includes #include "input.h" @@ -40,6 +41,29 @@ namespace video { int enableIntraRefresh; ///< Intra refresh setting: 0 = disabled, 1 = enabled. }; + namespace amf { + + /** + * @brief Enumerates supported coder options for the AMF encoder. + */ + enum class coder_e : int { + auto_ = 0, ///< Select the coder based on the H.264 profile. + cabac = 1, ///< CABAC entropy coding. + cavlc = 2, ///< CAVLC entropy coding. + }; + + } // namespace amf + + /** + * @brief Select the effective FFmpeg H.264 profile for an encoder configuration. + * + * @param encoder_name FFmpeg encoder name selected for the stream. + * @param config Encoding configuration requested by the remote client. + * @param amd_coder Configured AMF entropy-coder value. + * @return FFmpeg H.264 profile applied to the codec context. + */ + int select_h264_profile(std::string_view encoder_name, const config_t &config, int amd_coder); + /** * @brief Map an FFmpeg hardware device type to Sunshine's memory type. * diff --git a/tests/unit/test_video.cpp b/tests/unit/test_video.cpp index b9c926bf8..6ad2d3a83 100644 --- a/tests/unit/test_video.cpp +++ b/tests/unit/test_video.cpp @@ -2,10 +2,20 @@ * @file tests/unit/test_video.cpp * @brief Test src/video.*. */ +// test includes #include "../tests_common.h" +// standard includes +#include +#include +#include + +// local includes +#include #include +using namespace std::literals; + struct EncoderTest: PlatformTestSuite, testing::WithParamInterface { void SetUp() override { BaseTest::SetUp(); @@ -50,6 +60,41 @@ TEST_P(EncoderTest, ValidateEncoder) { // todo:: test something besides fixture setup } +/** + * @brief Parameterized coverage for effective H.264 profile selection. + */ +struct H264ProfileTest: testing::TestWithParam> {}; + +TEST_P(H264ProfileTest, SelectProfile) { + const auto &[encoder_name, coder, chroma_sampling_type, expected_profile] = GetParam(); + video::config_t config {}; + config.chromaSamplingType = chroma_sampling_type; + + EXPECT_EQ(expected_profile, video::select_h264_profile(encoder_name, config, std::to_underlying(coder))); +} + +INSTANTIATE_TEST_SUITE_P( + H264ProfileTests, + H264ProfileTest, + testing::Values( + std::make_tuple("h264_amf"sv, video::amf::coder_e::auto_, 0, AV_PROFILE_H264_HIGH), + std::make_tuple("h264_amf"sv, video::amf::coder_e::cabac, 0, AV_PROFILE_H264_HIGH), + std::make_tuple("h264_amf"sv, video::amf::coder_e::cavlc, 0, AV_PROFILE_H264_CONSTRAINED_BASELINE), + std::make_tuple("h264_amf"sv, video::amf::coder_e::cavlc, 1, AV_PROFILE_H264_HIGH_444_PREDICTIVE), + std::make_tuple("h264_nvenc"sv, video::amf::coder_e::cavlc, 0, AV_PROFILE_H264_HIGH) + ) +); + +#ifdef _WIN32 +TEST(AmfH264OptionsTest, CoderUsesConfiguredValue) { + const auto coder_option = std::ranges::find(video::amdvce.h264.common_options, "coder"sv, &video::encoder_t::option_t::name); + + ASSERT_NE(video::amdvce.h264.common_options.end(), coder_option); + ASSERT_TRUE(std::holds_alternative(coder_option->value)); + EXPECT_EQ(&config::video.amd.amd_coder, std::get(coder_option->value)); +} +#endif + struct FramerateX100Test: BaseTest, testing::WithParamInterface> {}; TEST_P(FramerateX100Test, Run) {