build: patch FFmpeg so that Direct3D 12 decoding works on AMD and Intel

.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

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

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;

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