Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use dynamic allocation for ffmpeg fft tables on Windows. #21539

Merged
merged 2 commits into from
Jan 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,6 @@ hooks = [
'--source-dir', '.',
'--filter', '^[0-9]\{{1,\}}\.[0-9]\{{1,\}}\.[0-9]\{{1,\}}$'],
},
{
'name': 'patch_ffmpeg',
'pattern': '.',
'action': ['vpython3', 'script/patch_ffmpeg.py'],
},
]

include_rules = [
Expand Down
6 changes: 5 additions & 1 deletion build/commands/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,25 @@ async function applyPatches() {
const v8PatchesPath = path.join(patchesPath, 'v8')
const catapultPatchesPath = path.join(patchesPath, 'third_party', 'catapult')
const devtoolsFrontendPatchesPath = path.join(patchesPath, 'third_party', 'devtools-frontend', 'src')
const ffmpegPatchesPath = path.join(patchesPath, 'third_party', 'ffmpeg')

const chromiumRepoPath = config.srcDir
const v8RepoPath = path.join(chromiumRepoPath, 'v8')
const catapultRepoPath = path.join(chromiumRepoPath, 'third_party', 'catapult')
const devtoolsFrontendRepoPath = path.join(chromiumRepoPath, 'third_party', 'devtools-frontend', 'src')
const ffmpegRepoPath = path.join(chromiumRepoPath, 'third_party', 'ffmpeg')

const chromiumPatcher = new GitPatcher(patchesPath, chromiumRepoPath)
const v8Patcher = new GitPatcher(v8PatchesPath, v8RepoPath)
const catapultPatcher = new GitPatcher(catapultPatchesPath, catapultRepoPath)
const devtoolsFrontendPatcher = new GitPatcher(devtoolsFrontendPatchesPath, devtoolsFrontendRepoPath)
const ffmpegPatcher = new GitPatcher(ffmpegPatchesPath, ffmpegRepoPath)

const chromiumPatchStatus = await chromiumPatcher.applyPatches()
const v8PatchStatus = await v8Patcher.applyPatches()
const catapultPatchStatus = await catapultPatcher.applyPatches()
const devtoolsFrontendPatchStatus = await devtoolsFrontendPatcher.applyPatches()
const ffmpegPatchStatus = await ffmpegPatcher.applyPatches()

// Log status for all patches
// Differentiate entries for logging
Expand All @@ -50,7 +54,7 @@ async function applyPatches() {
s => s.path = path.join('third_party', 'catapult', s.path))
devtoolsFrontendPatchStatus.forEach(
s => s.path = path.join('third_party', 'devtools-frontend', 'src', s.path))
const allPatchStatus = [...chromiumPatchStatus, ...v8PatchStatus, ...catapultPatchStatus, ...devtoolsFrontendPatchStatus]
const allPatchStatus = [...chromiumPatchStatus, ...v8PatchStatus, ...catapultPatchStatus, ...devtoolsFrontendPatchStatus, ...ffmpegPatchStatus]
Log.allPatchStatus(allPatchStatus, 'Chromium')

const hasPatchError = allPatchStatus.some(p => p.error)
Expand Down
4 changes: 4 additions & 0 deletions build/commands/scripts/updatePatches.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@ module.exports = function RunCommand (options) {
const v8Dir = path.join(config.srcDir, 'v8')
const catapultDir = path.join(config.srcDir, 'third_party', 'catapult')
const devtoolsFrontendDir = path.join(config.srcDir, 'third_party', 'devtools-frontend', 'src')
const ffmpegDir = path.join(config.srcDir, 'third_party', 'ffmpeg')
const patchDir = path.join(config.braveCoreDir, 'patches')
const v8PatchDir = path.join(patchDir, 'v8')
const catapultPatchDir = path.join(patchDir, 'third_party', 'catapult')
const devtoolsFrontendPatchDir = path.join(patchDir, 'third_party', 'devtools-frontend', 'src')
const ffmpegPatchDir = path.join(patchDir, 'third_party', 'ffmpeg')

Promise.all([
// chromium
Expand All @@ -36,6 +38,8 @@ module.exports = function RunCommand (options) {
updatePatches(catapultDir, catapultPatchDir),
// third_party/devtools-frontend/src
updatePatches(devtoolsFrontendDir, devtoolsFrontendPatchDir),
// third_party/ffmpeg
updatePatches(ffmpegDir, ffmpegPatchDir),
])
.then(() => {
console.log('Done.')
Expand Down
79 changes: 79 additions & 0 deletions patches/third_party/ffmpeg/libavutil-tx_template.c.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
diff --git a/libavutil/tx_template.c b/libavutil/tx_template.c
index 8dc3d2519c13abec090b374fa5ed0f11f7153849..a5d156d2789ec521c034edd929e04b2ad09a833b 100644
--- a/libavutil/tx_template.c
+++ b/libavutil/tx_template.c
@@ -27,6 +27,29 @@
#define TABLE_DEF(name, size) \
DECLARE_ALIGNED(32, TXSample, TX_TAB(ff_tx_tab_ ##name))[size]

+#ifdef _WIN32
+
+#include <malloc.h>
+
+// Declares aligned ff_tx_tab_* pointer.
+#define TABLE_DEF_PTR(name, size) \
+ DECLARE_ALIGNED(32, TXSample*, TX_TAB(ff_tx_tab_##name))
+
+// Allocates aligned memory for ff_tx_tab_* variable.
+#define ALLOCATE_FF_TX_TABLE(len) \
+ if (!TX_TAB(ff_tx_tab_##len)) { \
+ TX_TAB(ff_tx_tab_##len) = \
+ _aligned_malloc((len / 4 + 1) * sizeof(TXSample), 32); \
+ }
+
+#else
+
+// Use static arrays on non-Windows targets as usual.
+#define TABLE_DEF_PTR(name, size) TABLE_DEF(name, size)
+#define ALLOCATE_FF_TX_TABLE(len)
+
+#endif
+
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we cannot use chromium_src for this override, because tx_template.c is included into tx_float.c.

#define SR_POW2_TABLES \
SR_TABLE(8) \
SR_TABLE(16) \
@@ -43,13 +66,9 @@
SR_TABLE(32768) \
SR_TABLE(65536) \
SR_TABLE(131072) \
- SR_TABLE(262144) \
- SR_TABLE(524288) \
- SR_TABLE(1048576) \
- SR_TABLE(2097152) \
Copy link
Collaborator

@atuchin-m atuchin-m Jan 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add the link to the FFmpeg discussion about adding these lines. this PR.
It's hard to find it from the paches blame.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's added into description
image


#define SR_TABLE(len) \
- TABLE_DEF(len, len/4 + 1);
+ TABLE_DEF_PTR(len, len/4 + 1);
/* Power of two tables */
SR_POW2_TABLES
#undef SR_TABLE
@@ -68,6 +87,7 @@ typedef struct FFTabInitData {
static av_cold void TX_TAB(ff_tx_init_tab_ ##len)(void) \
{ \
double freq = 2*M_PI/len; \
+ ALLOCATE_FF_TX_TABLE(len); \
TXSample *tab = TX_TAB(ff_tx_tab_ ##len); \
\
for (int i = 0; i < len/4; i++) \
@@ -721,10 +741,6 @@ DECL_SR_CODELET(16384,8192,4096)
DECL_SR_CODELET(32768,16384,8192)
DECL_SR_CODELET(65536,32768,16384)
DECL_SR_CODELET(131072,65536,32768)
-DECL_SR_CODELET(262144,131072,65536)
-DECL_SR_CODELET(524288,262144,131072)
-DECL_SR_CODELET(1048576,524288,262144)
-DECL_SR_CODELET(2097152,1048576,524288)

static av_cold int TX_NAME(ff_tx_fft_init)(AVTXContext *s,
const FFTXCodelet *cd,
@@ -2157,10 +2173,6 @@ const FFTXCodelet * const TX_NAME(ff_tx_codelet_list)[] = {
&TX_NAME(ff_tx_fft32768_ns_def),
&TX_NAME(ff_tx_fft65536_ns_def),
&TX_NAME(ff_tx_fft131072_ns_def),
- &TX_NAME(ff_tx_fft262144_ns_def),
- &TX_NAME(ff_tx_fft524288_ns_def),
- &TX_NAME(ff_tx_fft1048576_ns_def),
- &TX_NAME(ff_tx_fft2097152_ns_def),

/* Prime factor codelets */
&TX_NAME(ff_tx_fft3_ns_def),
39 changes: 0 additions & 39 deletions script/patch_ffmpeg.py

This file was deleted.