From c5d35ebd2076d1f0b00161c6c45c61077a760d86 Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Mon, 5 Oct 2026 22:48:47 +0200 Subject: [PATCH] build: patch FFmpeg so that Direct3D 12 decoding works on AMD and Intel FFmpeg 8.1 cannot decode through Direct3D 12 for long on a driver that needs reference-only allocations: it takes a second reference-only resource each time its frame pool hands out a texture it already has one for, nothing frees the extra, and the decoder fails with "No space for new Reference frame!" after as many pictures as it has reference slots, 10 for VP9 and 18 for H.264. Since the media player tries Direct3D 12 first, hardware decoding fell back to software on an AMD Radeon RX 9070 XT for every stream, and 8 tests of HardwareDecodingTest failed where hardware is required. patches/0001-d3d12va-reuse-the-reference-only-resource-of-a-texture.patch makes a texture keep the resource it has, and makes the loops over the map cover all the entries it is allocated with. With it the same 27 tests pass on that GPU with Direct3D 12 first, and a 14315 frame 1080p H.264 stream decodes to its end. The build applies the patches in that directory to the submodule for the length of the FFmpeg build and takes them off again, so the submodule is clean afterwards, whether the build worked or not. A patch that does not apply stops the build, which is how a bump of FFmpeg shows that the fix has arrived and the patch can go. Their names and hashes are part of components.txt, and of the key of the CI cache, so that an install built without a patch is built again. --- .github/actions/build/action.yml | 4 +- .../cpp/dependencies/ffmpeg/CMakeLists.txt | 120 +++++++++++++++++- ...reference-only-resource-of-a-texture.patch | 90 +++++++++++++ .../cpp/dependencies/ffmpeg/patches/README.md | 16 +++ 4 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 webrtc-java-media/src/main/cpp/dependencies/ffmpeg/patches/0001-d3d12va-reuse-the-reference-only-resource-of-a-texture.patch create mode 100644 webrtc-java-media/src/main/cpp/dependencies/ffmpeg/patches/README.md diff --git a/.github/actions/build/action.yml b/.github/actions/build/action.yml index cbd8fde4e..83692d53d 100644 --- a/.github/actions/build/action.yml +++ b/.github/actions/build/action.yml @@ -50,8 +50,8 @@ runs: uses: actions/cache/restore@v6 with: path: ~/ffmpeg - key: ffmpeg-${{ env.FFMPEG_VERSION }}-${{ inputs.platform-name }}-${{ hashFiles('webrtc-java-media/src/main/cpp/dependencies/ffmpeg/CMakeLists.txt') }}-${{ hashFiles('webrtc-java-media/pom.xml') }} - restore-keys: ffmpeg-${{ env.FFMPEG_VERSION }}-${{ inputs.platform-name }}-${{ hashFiles('webrtc-java-media/src/main/cpp/dependencies/ffmpeg/CMakeLists.txt') }}- + key: ffmpeg-${{ env.FFMPEG_VERSION }}-${{ inputs.platform-name }}-${{ hashFiles('webrtc-java-media/src/main/cpp/dependencies/ffmpeg/CMakeLists.txt', 'webrtc-java-media/src/main/cpp/dependencies/ffmpeg/patches/*.patch') }}-${{ hashFiles('webrtc-java-media/pom.xml') }} + restore-keys: ffmpeg-${{ env.FFMPEG_VERSION }}-${{ inputs.platform-name }}-${{ hashFiles('webrtc-java-media/src/main/cpp/dependencies/ffmpeg/CMakeLists.txt', 'webrtc-java-media/src/main/cpp/dependencies/ffmpeg/patches/*.patch') }}- # One cache per OS: the dependencies do not differ between the platforms of # an OS. The project's own artifacts change with every build and stay out diff --git a/webrtc-java-media/src/main/cpp/dependencies/ffmpeg/CMakeLists.txt b/webrtc-java-media/src/main/cpp/dependencies/ffmpeg/CMakeLists.txt index 03dd875b6..9f766860d 100644 --- a/webrtc-java-media/src/main/cpp/dependencies/ffmpeg/CMakeLists.txt +++ b/webrtc-java-media/src/main/cpp/dependencies/ffmpeg/CMakeLists.txt @@ -184,6 +184,108 @@ set(FFMPEG_NVDEC "${FFMPEG_NVDEC}" PARENT_SCOPE) set(FFMPEG_COMPONENTS_FILE "${FFMPEG_INSTALL_DIR}/components.txt") string(REPLACE ";" "\n" FFMPEG_COMPONENTS_TEXT "${FFMPEG_VERSION};${FFMPEG_COMPONENTS}") +# +# Fixes to FFmpeg that its release does not have yet, in patches/*.patch, in +# the order of their names. They are applied to the source for the build and +# taken off it again afterwards, so that the submodule is as clean after a +# build as it was before, and a bump of the submodule shows at once which of +# them its release still needs: one that no longer applies stops the build. +# +# They belong to the install like its components do, by the name and the +# contents of each, so that an install built without a patch, or with another +# version of it, is built again. +# +file(GLOB FFMPEG_PATCHES "${CMAKE_CURRENT_SOURCE_DIR}/patches/*.patch") +list(SORT FFMPEG_PATCHES) + +foreach(PATCH ${FFMPEG_PATCHES}) + get_filename_component(PATCH_NAME "${PATCH}" NAME) + file(SHA256 "${PATCH}" PATCH_HASH) + + string(APPEND FFMPEG_COMPONENTS_TEXT "\npatch ${PATCH_NAME} ${PATCH_HASH}") +endforeach() + +# Applies the patches to the source. One that is applied already is left as it +# is: a build that was cut short does not get to take them off. +function(ffmpeg_apply_patches) + foreach(PATCH ${FFMPEG_PATCHES}) + get_filename_component(PATCH_NAME "${PATCH}" NAME) + + # Whitespace is ignored because a checkout may have turned the line + # endings of the source into something else than the patch has. + execute_process( + COMMAND "${GIT_EXECUTABLE}" apply --check --ignore-whitespace "${PATCH}" + WORKING_DIRECTORY "${FFMPEG_SOURCE_DIR}" + RESULT_VARIABLE CAN_APPLY + OUTPUT_QUIET ERROR_QUIET) + + if(CAN_APPLY EQUAL 0) + execute_process( + COMMAND "${GIT_EXECUTABLE}" apply --ignore-whitespace "${PATCH}" + WORKING_DIRECTORY "${FFMPEG_SOURCE_DIR}" + RESULT_VARIABLE APPLY_RESULT) + + if(NOT APPLY_RESULT EQUAL 0) + ffmpeg_revert_patches() + + message(FATAL_ERROR "Applying ${PATCH_NAME} to the FFmpeg source failed (${APPLY_RESULT})") + endif() + + message(STATUS "Applied ${PATCH_NAME} to the FFmpeg source") + continue() + endif() + + execute_process( + COMMAND "${GIT_EXECUTABLE}" apply --check --reverse --ignore-whitespace "${PATCH}" + WORKING_DIRECTORY "${FFMPEG_SOURCE_DIR}" + RESULT_VARIABLE IS_APPLIED + OUTPUT_QUIET ERROR_QUIET) + + if(NOT IS_APPLIED EQUAL 0) + # The ones before it come off again: the source is left as it was. + ffmpeg_revert_patches() + + message(FATAL_ERROR + "${PATCH_NAME} does not apply to the FFmpeg source in ${FFMPEG_SOURCE_DIR}. If the " + "submodule moved to a release that has the fix, remove the patch from " + "${CMAKE_CURRENT_SOURCE_DIR}/patches; otherwise update it.") + endif() + + message(STATUS "${PATCH_NAME} is applied to the FFmpeg source already") + endforeach() +endfunction() + +# Takes the patches that are on the source off it again, last one first. +function(ffmpeg_revert_patches) + set(REVERSED_PATCHES ${FFMPEG_PATCHES}) + list(REVERSE REVERSED_PATCHES) + + foreach(PATCH ${REVERSED_PATCHES}) + get_filename_component(PATCH_NAME "${PATCH}" NAME) + + execute_process( + COMMAND "${GIT_EXECUTABLE}" apply --check --reverse --ignore-whitespace "${PATCH}" + WORKING_DIRECTORY "${FFMPEG_SOURCE_DIR}" + RESULT_VARIABLE IS_ON + OUTPUT_QUIET ERROR_QUIET) + + if(NOT IS_ON EQUAL 0) + continue() + endif() + + execute_process( + COMMAND "${GIT_EXECUTABLE}" apply --reverse --ignore-whitespace "${PATCH}" + WORKING_DIRECTORY "${FFMPEG_SOURCE_DIR}" + RESULT_VARIABLE REVERT_RESULT) + + if(NOT REVERT_RESULT EQUAL 0) + message(WARNING + "Taking ${PATCH_NAME} off the FFmpeg source failed (${REVERT_RESULT}); the submodule " + "is left modified. `git checkout .` in ${FFMPEG_SOURCE_DIR} restores it.") + endif() + endforeach() +endfunction() + # # Decides whether FFmpeg has to be built, by looking for one of its headers # and one of its libraries in the install directory, and for a record of it @@ -489,7 +591,8 @@ else() list(APPEND FFMPEG_CONFIGURE_ARGS "--extra-ldflags=\"${FFMPEG_EXTRA_LDFLAGS}\"") endif() - # Build out of tree so the submodule working tree stays clean. + # Build out of tree so the submodule working tree stays clean. The patches + # are put on it for the length of the build only. set(FFMPEG_BUILD_DIR "${CMAKE_CURRENT_BINARY_DIR}/ffmpeg-build") # A rebuild starts from nothing. make install only adds to the install @@ -607,12 +710,27 @@ set MSYS2_PATH_TYPE=inherit message(STATUS "Building FFmpeg ${FFMPEG_VERSION} into ${FFMPEG_INSTALL_DIR} with ${BUILD_JOBS} jobs") message(STATUS " configure ${FFMPEG_CONFIGURE_ARGS_STR}") + if(FFMPEG_PATCHES) + find_program(GIT_EXECUTABLE NAMES git) + + if(NOT GIT_EXECUTABLE) + message(FATAL_ERROR "No 'git' found, which applies the patches to FFmpeg in ${CMAKE_CURRENT_SOURCE_DIR}/patches.") + endif() + + ffmpeg_apply_patches() + endif() + execute_process( COMMAND ${FFMPEG_BUILD_COMMAND} WORKING_DIRECTORY "${FFMPEG_BUILD_DIR}" RESULT_VARIABLE BUILD_RESULT ) + # Off again whether or not the build worked. + if(FFMPEG_PATCHES) + ffmpeg_revert_patches() + endif() + if(NOT BUILD_RESULT EQUAL 0) # configure says almost nothing on the console and writes the actual # reason into config.log, which on a build machine nobody can reach is diff --git a/webrtc-java-media/src/main/cpp/dependencies/ffmpeg/patches/0001-d3d12va-reuse-the-reference-only-resource-of-a-texture.patch b/webrtc-java-media/src/main/cpp/dependencies/ffmpeg/patches/0001-d3d12va-reuse-the-reference-only-resource-of-a-texture.patch new file mode 100644 index 000000000..b482e47c2 --- /dev/null +++ b/webrtc-java-media/src/main/cpp/dependencies/ffmpeg/patches/0001-d3d12va-reuse-the-reference-only-resource-of-a-texture.patch @@ -0,0 +1,90 @@ +d3d12va: reuse the reference-only resource of a texture that is decoded into again + +Applies to FFmpeg n8.1. Not needed once FFmpeg has its own fix: drop this +file when a release that has one is pinned. + +On a driver that requires reference-only allocations (AMD, Intel), +d3d12va_decode.c keeps a map of reference-only resources next to the pictures +it decodes, and frees an entry only once the texture it was made for is no +longer listed in ref_resources. A hardware frame pool hands out the same few +textures again and again, and a texture that is handed out again is, as a +rule, still listed there, as the entry of a picture that is long gone. Each +time, get_reference_only_resource() took a free entry for it, or made a new +one, though the texture had one already, and nothing freed the extra: both +entries match the texture, so prepare_reference_only_resources() keeps both, +and it hands the decoder the first it finds, which is the older one, holding +a picture from long ago, as the reference of the new ones. + +Decoding then fails with "No space for new Reference frame!" once the entries +run out: at the 10th picture of a VP9 stream and the 18th of an H.264 one, +with the pool and reference sets of the small streams this was found with. +Until then the decoder can be given the older entry, and so a stale picture, +as a reference. + +The second problem is that the map is allocated with max_num_ref + 1 +entries, one more than a picture can have references, because the entry of +the picture that has just left the reference set is only freed after the new +one has been given its own. Every loop over the map stops one short of that. + +A texture that already has an entry now keeps it, and the loops cover the +whole map. + +diff --git a/libavcodec/d3d12va_decode.c b/libavcodec/d3d12va_decode.c +index 4e714efb1a..3e34d7583c 100644 +--- a/libavcodec/d3d12va_decode.c ++++ b/libavcodec/d3d12va_decode.c +@@ -61,8 +61,20 @@ static ID3D12Resource *get_reference_only_resource(AVCodecContext *avctx, ID3D12 + return NULL; + } + ++ // A texture that is decoded into again, because the pool handed it out ++ // once more, keeps the reference-only resource it had. Giving it another ++ // would leave two for one texture, which nothing frees as long as the ++ // texture is listed in ref_resources, and which prepare_reference_only_resources ++ // would pick between by order rather than by age. ++ for (i = 0; i < ctx->max_num_ref + 1; i++) { ++ if (reference_only_map[i].resource != NULL && reference_only_map[i].output_resource == output_resource) { ++ reference_only_map[i].used = 1; ++ return reference_only_map[i].resource; ++ } ++ } ++ + // find unused resource +- for (i = 0; i < ctx->max_num_ref; i++) { ++ for (i = 0; i < ctx->max_num_ref + 1; i++) { + if (!reference_only_map[i].used && reference_only_map[i].resource != NULL) { + reference_only_map[i].used = 1; + resource = reference_only_map[i].resource; +@@ -72,12 +84,12 @@ static ID3D12Resource *get_reference_only_resource(AVCodecContext *avctx, ID3D12 + } + + // find space to allocate +- for (i = 0; i < ctx->max_num_ref; i++) { ++ for (i = 0; i < ctx->max_num_ref + 1; i++) { + if (reference_only_map[i].resource == NULL) + break; + } + +- if (i == ctx->max_num_ref) { ++ if (i == ctx->max_num_ref + 1) { + av_log(avctx, AV_LOG_ERROR, "No space for new Reference frame!\n"); + return NULL; + } +@@ -105,7 +117,7 @@ static void free_reference_only_resources(AVCodecContext *avctx) + int i; + ReferenceFrame *reference_only_map = ctx->reference_only_map; + if (reference_only_map != NULL) { +- for (i = 0; i < ctx->max_num_ref; i++) { ++ for (i = 0; i < ctx->max_num_ref + 1; i++) { + if (reference_only_map[i].resource != NULL) { + D3D12_OBJECT_RELEASE(reference_only_map[i].resource); + } +@@ -123,7 +135,7 @@ static void prepare_reference_only_resources(AVCodecContext *avctx) + if (reference_only_map == NULL) + return; + memset(ctx->ref_only_resources, 0, ctx->max_num_ref * sizeof(*(ctx->ref_only_resources))); +- for (j = 0; j < ctx->max_num_ref; j++) { ++ for (j = 0; j < ctx->max_num_ref + 1; j++) { + for (i = 0; i < ctx->max_num_ref; i++) { + if (reference_only_map[j].used && reference_only_map[j].output_resource == ctx->ref_resources[i]) { + ctx->ref_only_resources[i] = reference_only_map[j].resource; diff --git a/webrtc-java-media/src/main/cpp/dependencies/ffmpeg/patches/README.md b/webrtc-java-media/src/main/cpp/dependencies/ffmpeg/patches/README.md new file mode 100644 index 000000000..269515bfb --- /dev/null +++ b/webrtc-java-media/src/main/cpp/dependencies/ffmpeg/patches/README.md @@ -0,0 +1,16 @@ +# Patches to FFmpeg + +Fixes that the pinned FFmpeg release does not have. `CMakeLists.txt` in the directory above applies every +`*.patch` here, in the order of their names, to the `third-party/ffmpeg` submodule for the length of the FFmpeg +build and takes them off again afterwards, so the submodule is clean once the build is done. + +- A patch is made against the release the submodule is at: `git diff` in the submodule, with its header above the + `diff --git` line saying what is wrong and why, which `git apply` ignores. +- The name and the contents of each patch are recorded in the `components.txt` of an install, and go into the key of + the CI cache, so adding, changing or removing one builds FFmpeg again. +- A patch that no longer applies stops the build. When the submodule moves to a new release, check whether it has + the fix, and remove the patch if it has. + +| Patch | Fixes | +| --- | --- | +| `0001-d3d12va-reuse-the-reference-only-resource-of-a-texture.patch` | Direct3D 12 decoding failing after the first few pictures on drivers that need reference-only allocations (AMD, Intel) |