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