Skip to content

Commit 4d205e9

Browse files
authored
build: patch FFmpeg so that Direct3D 12 decoding works on AMD and Intel (#330)
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.
1 parent 997f4fe commit 4d205e9

4 files changed

Lines changed: 227 additions & 3 deletions

File tree

‎.github/actions/build/action.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ runs:
5050
uses: actions/cache/restore@v6
5151
with:
5252
path: ~/ffmpeg
53-
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') }}
54-
restore-keys: ffmpeg-${{ env.FFMPEG_VERSION }}-${{ inputs.platform-name }}-${{ hashFiles('webrtc-java-media/src/main/cpp/dependencies/ffmpeg/CMakeLists.txt') }}-
53+
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') }}
54+
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') }}-
5555

5656
# One cache per OS: the dependencies do not differ between the platforms of
5757
# an OS. The project's own artifacts change with every build and stay out

‎webrtc-java-media/src/main/cpp/dependencies/ffmpeg/CMakeLists.txt‎

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,108 @@ set(FFMPEG_NVDEC "${FFMPEG_NVDEC}" PARENT_SCOPE)
184184
set(FFMPEG_COMPONENTS_FILE "${FFMPEG_INSTALL_DIR}/components.txt")
185185
string(REPLACE ";" "\n" FFMPEG_COMPONENTS_TEXT "${FFMPEG_VERSION};${FFMPEG_COMPONENTS}")
186186

187+
#
188+
# Fixes to FFmpeg that its release does not have yet, in patches/*.patch, in
189+
# the order of their names. They are applied to the source for the build and
190+
# taken off it again afterwards, so that the submodule is as clean after a
191+
# build as it was before, and a bump of the submodule shows at once which of
192+
# them its release still needs: one that no longer applies stops the build.
193+
#
194+
# They belong to the install like its components do, by the name and the
195+
# contents of each, so that an install built without a patch, or with another
196+
# version of it, is built again.
197+
#
198+
file(GLOB FFMPEG_PATCHES "${CMAKE_CURRENT_SOURCE_DIR}/patches/*.patch")
199+
list(SORT FFMPEG_PATCHES)
200+
201+
foreach(PATCH ${FFMPEG_PATCHES})
202+
get_filename_component(PATCH_NAME "${PATCH}" NAME)
203+
file(SHA256 "${PATCH}" PATCH_HASH)
204+
205+
string(APPEND FFMPEG_COMPONENTS_TEXT "\npatch ${PATCH_NAME} ${PATCH_HASH}")
206+
endforeach()
207+
208+
# Applies the patches to the source. One that is applied already is left as it
209+
# is: a build that was cut short does not get to take them off.
210+
function(ffmpeg_apply_patches)
211+
foreach(PATCH ${FFMPEG_PATCHES})
212+
get_filename_component(PATCH_NAME "${PATCH}" NAME)
213+
214+
# Whitespace is ignored because a checkout may have turned the line
215+
# endings of the source into something else than the patch has.
216+
execute_process(
217+
COMMAND "${GIT_EXECUTABLE}" apply --check --ignore-whitespace "${PATCH}"
218+
WORKING_DIRECTORY "${FFMPEG_SOURCE_DIR}"
219+
RESULT_VARIABLE CAN_APPLY
220+
OUTPUT_QUIET ERROR_QUIET)
221+
222+
if(CAN_APPLY EQUAL 0)
223+
execute_process(
224+
COMMAND "${GIT_EXECUTABLE}" apply --ignore-whitespace "${PATCH}"
225+
WORKING_DIRECTORY "${FFMPEG_SOURCE_DIR}"
226+
RESULT_VARIABLE APPLY_RESULT)
227+
228+
if(NOT APPLY_RESULT EQUAL 0)
229+
ffmpeg_revert_patches()
230+
231+
message(FATAL_ERROR "Applying ${PATCH_NAME} to the FFmpeg source failed (${APPLY_RESULT})")
232+
endif()
233+
234+
message(STATUS "Applied ${PATCH_NAME} to the FFmpeg source")
235+
continue()
236+
endif()
237+
238+
execute_process(
239+
COMMAND "${GIT_EXECUTABLE}" apply --check --reverse --ignore-whitespace "${PATCH}"
240+
WORKING_DIRECTORY "${FFMPEG_SOURCE_DIR}"
241+
RESULT_VARIABLE IS_APPLIED
242+
OUTPUT_QUIET ERROR_QUIET)
243+
244+
if(NOT IS_APPLIED EQUAL 0)
245+
# The ones before it come off again: the source is left as it was.
246+
ffmpeg_revert_patches()
247+
248+
message(FATAL_ERROR
249+
"${PATCH_NAME} does not apply to the FFmpeg source in ${FFMPEG_SOURCE_DIR}. If the "
250+
"submodule moved to a release that has the fix, remove the patch from "
251+
"${CMAKE_CURRENT_SOURCE_DIR}/patches; otherwise update it.")
252+
endif()
253+
254+
message(STATUS "${PATCH_NAME} is applied to the FFmpeg source already")
255+
endforeach()
256+
endfunction()
257+
258+
# Takes the patches that are on the source off it again, last one first.
259+
function(ffmpeg_revert_patches)
260+
set(REVERSED_PATCHES ${FFMPEG_PATCHES})
261+
list(REVERSE REVERSED_PATCHES)
262+
263+
foreach(PATCH ${REVERSED_PATCHES})
264+
get_filename_component(PATCH_NAME "${PATCH}" NAME)
265+
266+
execute_process(
267+
COMMAND "${GIT_EXECUTABLE}" apply --check --reverse --ignore-whitespace "${PATCH}"
268+
WORKING_DIRECTORY "${FFMPEG_SOURCE_DIR}"
269+
RESULT_VARIABLE IS_ON
270+
OUTPUT_QUIET ERROR_QUIET)
271+
272+
if(NOT IS_ON EQUAL 0)
273+
continue()
274+
endif()
275+
276+
execute_process(
277+
COMMAND "${GIT_EXECUTABLE}" apply --reverse --ignore-whitespace "${PATCH}"
278+
WORKING_DIRECTORY "${FFMPEG_SOURCE_DIR}"
279+
RESULT_VARIABLE REVERT_RESULT)
280+
281+
if(NOT REVERT_RESULT EQUAL 0)
282+
message(WARNING
283+
"Taking ${PATCH_NAME} off the FFmpeg source failed (${REVERT_RESULT}); the submodule "
284+
"is left modified. `git checkout .` in ${FFMPEG_SOURCE_DIR} restores it.")
285+
endif()
286+
endforeach()
287+
endfunction()
288+
187289
#
188290
# Decides whether FFmpeg has to be built, by looking for one of its headers
189291
# and one of its libraries in the install directory, and for a record of it
@@ -489,7 +591,8 @@ else()
489591
list(APPEND FFMPEG_CONFIGURE_ARGS "--extra-ldflags=\"${FFMPEG_EXTRA_LDFLAGS}\"")
490592
endif()
491593

492-
# Build out of tree so the submodule working tree stays clean.
594+
# Build out of tree so the submodule working tree stays clean. The patches
595+
# are put on it for the length of the build only.
493596
set(FFMPEG_BUILD_DIR "${CMAKE_CURRENT_BINARY_DIR}/ffmpeg-build")
494597

495598
# A rebuild starts from nothing. make install only adds to the install
@@ -607,12 +710,27 @@ set MSYS2_PATH_TYPE=inherit
607710
message(STATUS "Building FFmpeg ${FFMPEG_VERSION} into ${FFMPEG_INSTALL_DIR} with ${BUILD_JOBS} jobs")
608711
message(STATUS " configure ${FFMPEG_CONFIGURE_ARGS_STR}")
609712

713+
if(FFMPEG_PATCHES)
714+
find_program(GIT_EXECUTABLE NAMES git)
715+
716+
if(NOT GIT_EXECUTABLE)
717+
message(FATAL_ERROR "No 'git' found, which applies the patches to FFmpeg in ${CMAKE_CURRENT_SOURCE_DIR}/patches.")
718+
endif()
719+
720+
ffmpeg_apply_patches()
721+
endif()
722+
610723
execute_process(
611724
COMMAND ${FFMPEG_BUILD_COMMAND}
612725
WORKING_DIRECTORY "${FFMPEG_BUILD_DIR}"
613726
RESULT_VARIABLE BUILD_RESULT
614727
)
615728

729+
# Off again whether or not the build worked.
730+
if(FFMPEG_PATCHES)
731+
ffmpeg_revert_patches()
732+
endif()
733+
616734
if(NOT BUILD_RESULT EQUAL 0)
617735
# configure says almost nothing on the console and writes the actual
618736
# reason into config.log, which on a build machine nobody can reach is
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
d3d12va: reuse the reference-only resource of a texture that is decoded into again
2+
3+
Applies to FFmpeg n8.1. Not needed once FFmpeg has its own fix: drop this
4+
file when a release that has one is pinned.
5+
6+
On a driver that requires reference-only allocations (AMD, Intel),
7+
d3d12va_decode.c keeps a map of reference-only resources next to the pictures
8+
it decodes, and frees an entry only once the texture it was made for is no
9+
longer listed in ref_resources. A hardware frame pool hands out the same few
10+
textures again and again, and a texture that is handed out again is, as a
11+
rule, still listed there, as the entry of a picture that is long gone. Each
12+
time, get_reference_only_resource() took a free entry for it, or made a new
13+
one, though the texture had one already, and nothing freed the extra: both
14+
entries match the texture, so prepare_reference_only_resources() keeps both,
15+
and it hands the decoder the first it finds, which is the older one, holding
16+
a picture from long ago, as the reference of the new ones.
17+
18+
Decoding then fails with "No space for new Reference frame!" once the entries
19+
run out: at the 10th picture of a VP9 stream and the 18th of an H.264 one,
20+
with the pool and reference sets of the small streams this was found with.
21+
Until then the decoder can be given the older entry, and so a stale picture,
22+
as a reference.
23+
24+
The second problem is that the map is allocated with max_num_ref + 1
25+
entries, one more than a picture can have references, because the entry of
26+
the picture that has just left the reference set is only freed after the new
27+
one has been given its own. Every loop over the map stops one short of that.
28+
29+
A texture that already has an entry now keeps it, and the loops cover the
30+
whole map.
31+
32+
diff --git a/libavcodec/d3d12va_decode.c b/libavcodec/d3d12va_decode.c
33+
index 4e714efb1a..3e34d7583c 100644
34+
--- a/libavcodec/d3d12va_decode.c
35+
+++ b/libavcodec/d3d12va_decode.c
36+
@@ -61,8 +61,20 @@ static ID3D12Resource *get_reference_only_resource(AVCodecContext *avctx, ID3D12
37+
return NULL;
38+
}
39+
40+
+ // A texture that is decoded into again, because the pool handed it out
41+
+ // once more, keeps the reference-only resource it had. Giving it another
42+
+ // would leave two for one texture, which nothing frees as long as the
43+
+ // texture is listed in ref_resources, and which prepare_reference_only_resources
44+
+ // would pick between by order rather than by age.
45+
+ for (i = 0; i < ctx->max_num_ref + 1; i++) {
46+
+ if (reference_only_map[i].resource != NULL && reference_only_map[i].output_resource == output_resource) {
47+
+ reference_only_map[i].used = 1;
48+
+ return reference_only_map[i].resource;
49+
+ }
50+
+ }
51+
+
52+
// find unused resource
53+
- for (i = 0; i < ctx->max_num_ref; i++) {
54+
+ for (i = 0; i < ctx->max_num_ref + 1; i++) {
55+
if (!reference_only_map[i].used && reference_only_map[i].resource != NULL) {
56+
reference_only_map[i].used = 1;
57+
resource = reference_only_map[i].resource;
58+
@@ -72,12 +84,12 @@ static ID3D12Resource *get_reference_only_resource(AVCodecContext *avctx, ID3D12
59+
}
60+
61+
// find space to allocate
62+
- for (i = 0; i < ctx->max_num_ref; i++) {
63+
+ for (i = 0; i < ctx->max_num_ref + 1; i++) {
64+
if (reference_only_map[i].resource == NULL)
65+
break;
66+
}
67+
68+
- if (i == ctx->max_num_ref) {
69+
+ if (i == ctx->max_num_ref + 1) {
70+
av_log(avctx, AV_LOG_ERROR, "No space for new Reference frame!\n");
71+
return NULL;
72+
}
73+
@@ -105,7 +117,7 @@ static void free_reference_only_resources(AVCodecContext *avctx)
74+
int i;
75+
ReferenceFrame *reference_only_map = ctx->reference_only_map;
76+
if (reference_only_map != NULL) {
77+
- for (i = 0; i < ctx->max_num_ref; i++) {
78+
+ for (i = 0; i < ctx->max_num_ref + 1; i++) {
79+
if (reference_only_map[i].resource != NULL) {
80+
D3D12_OBJECT_RELEASE(reference_only_map[i].resource);
81+
}
82+
@@ -123,7 +135,7 @@ static void prepare_reference_only_resources(AVCodecContext *avctx)
83+
if (reference_only_map == NULL)
84+
return;
85+
memset(ctx->ref_only_resources, 0, ctx->max_num_ref * sizeof(*(ctx->ref_only_resources)));
86+
- for (j = 0; j < ctx->max_num_ref; j++) {
87+
+ for (j = 0; j < ctx->max_num_ref + 1; j++) {
88+
for (i = 0; i < ctx->max_num_ref; i++) {
89+
if (reference_only_map[j].used && reference_only_map[j].output_resource == ctx->ref_resources[i]) {
90+
ctx->ref_only_resources[i] = reference_only_map[j].resource;
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# Patches to FFmpeg
2+
3+
Fixes that the pinned FFmpeg release does not have. `CMakeLists.txt` in the directory above applies every
4+
`*.patch` here, in the order of their names, to the `third-party/ffmpeg` submodule for the length of the FFmpeg
5+
build and takes them off again afterwards, so the submodule is clean once the build is done.
6+
7+
- A patch is made against the release the submodule is at: `git diff` in the submodule, with its header above the
8+
`diff --git` line saying what is wrong and why, which `git apply` ignores.
9+
- The name and the contents of each patch are recorded in the `components.txt` of an install, and go into the key of
10+
the CI cache, so adding, changing or removing one builds FFmpeg again.
11+
- A patch that no longer applies stops the build. When the submodule moves to a new release, check whether it has
12+
the fix, and remove the patch if it has.
13+
14+
| Patch | Fixes |
15+
| --- | --- |
16+
| `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) |

0 commit comments

Comments
 (0)