Skip to content

Commit 84ee7e4

Browse files
D-Ogiclaude
andcommitted
Reuse GPU textures between chained placebo filters
When multiple placebo filters are stacked on one clip, each filter previously did a full RAM→GPU upload and GPU→RAM download. The intermediate uploads are redundant because the next placebo filter would re-upload the same pixels immediately. Each filter now attaches its output texture to the mlt_frame via placebo_frame_put_tex(). The next placebo filter calls placebo_frame_take_tex() to grab it directly as source, skipping the upload. The download to RAM still happens every time (MLT expects the image buffer to be current for non-GPU filters). Staleness detection: put_tex records the RAM buffer pointer, take_tex compares it against the current pointer. If a CPU filter ran in between and requested a writable buffer (triggering a copy and new allocation), the pointers differ and take_tex returns NULL, falling back to a fresh upload. Also cleans up internal ticket-style comments (C1/W2/etc.) with descriptions of actual logic and pitfalls. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 394d73c commit 84ee7e4

4 files changed

Lines changed: 178 additions & 65 deletions

File tree

‎src/modules/placebo/filter_placebo_render.c‎

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ static int filter_get_image(mlt_frame frame,
9999
return mlt_frame_get_image(frame, image, format, width, height, writable);
100100
}
101101

102-
/* W2 fix: verify RGBA8 format is supported */
102+
/* Not all GPU backends guarantee rgba8 support (e.g. some OpenGL ES) */
103103
pl_fmt rgba_fmt = pl_find_named_fmt(gpu, "rgba8");
104104
if (!rgba_fmt) {
105105
mlt_log_error(MLT_FILTER_SERVICE(filter), "GPU does not support rgba8 format\n");
@@ -116,38 +116,48 @@ static int filter_get_image(mlt_frame frame,
116116
int h = *height;
117117
size_t stride = w * 4;
118118

119-
/* W4 fix: hold render lock for all GPU operations */
119+
/* pl_renderer is not thread-safe; hold the lock for the entire
120+
* upload → render → download sequence */
120121
placebo_render_lock();
121122

122-
/* Create source texture and upload */
123-
pl_tex src_tex = pl_tex_create(gpu,
124-
pl_tex_params(.w = w,
125-
.h = h,
126-
.format = rgba_fmt,
127-
.sampleable = true,
128-
.host_writable = true, ));
129-
if (!src_tex) {
130-
mlt_log_error(MLT_FILTER_SERVICE(filter), "Failed to create source texture\n");
131-
placebo_render_unlock();
132-
return 0; /* pass through original frame data */
133-
}
123+
/* Try to reuse a GPU texture left by a preceding placebo filter on
124+
* this frame. Returns NULL if there is none, or if the RAM buffer was
125+
* reallocated by an intervening CPU filter (stale texture). */
126+
pl_tex reused_src = placebo_frame_take_tex(frame, *image);
127+
pl_tex src_tex;
128+
if (reused_src) {
129+
src_tex = reused_src;
130+
} else {
131+
src_tex = pl_tex_create(gpu,
132+
pl_tex_params(.w = w,
133+
.h = h,
134+
.format = rgba_fmt,
135+
.sampleable = true,
136+
.host_writable = true, ));
137+
if (!src_tex) {
138+
mlt_log_error(MLT_FILTER_SERVICE(filter), "Failed to create source texture\n");
139+
placebo_render_unlock();
140+
return 0;
141+
}
134142

135-
/* C4 fix: check upload return */
136-
if (!pl_tex_upload(gpu,
137-
pl_tex_transfer_params(.tex = src_tex,
138-
.row_pitch = stride,
139-
.ptr = *image, ))) {
140-
mlt_log_error(MLT_FILTER_SERVICE(filter), "GPU texture upload failed\n");
141-
pl_tex_destroy(gpu, &src_tex);
142-
placebo_render_unlock();
143-
return 0;
143+
if (!pl_tex_upload(gpu,
144+
pl_tex_transfer_params(.tex = src_tex,
145+
.row_pitch = stride,
146+
.ptr = *image, ))) {
147+
mlt_log_error(MLT_FILTER_SERVICE(filter), "GPU texture upload failed\n");
148+
pl_tex_destroy(gpu, &src_tex);
149+
placebo_render_unlock();
150+
return 0;
151+
}
144152
}
145153

146-
/* Create destination texture */
154+
/* sampleable is needed so a subsequent placebo filter can bind this
155+
* texture as its source without re-uploading from RAM. */
147156
pl_tex dst_tex = pl_tex_create(gpu,
148157
pl_tex_params(.w = w,
149158
.h = h,
150159
.format = rgba_fmt,
160+
.sampleable = true,
151161
.renderable = true,
152162
.host_readable = true, ));
153163
if (!dst_tex) {
@@ -228,17 +238,20 @@ static int filter_get_image(mlt_frame frame,
228238
mlt_log_warning(MLT_FILTER_SERVICE(filter), "pl_render_image failed\n");
229239
}
230240

231-
/* C4 fix: check download return */
241+
/* Always download to RAM — MLT expects *image to hold current pixels,
242+
* even though the texture may be reused on the GPU side. */
232243
if (!pl_tex_download(gpu,
233244
pl_tex_transfer_params(.tex = dst_tex,
234245
.row_pitch = stride,
235246
.ptr = *image, ))) {
236247
mlt_log_warning(MLT_FILTER_SERVICE(filter), "GPU texture download failed\n");
237248
}
238249

239-
/* Cleanup textures */
240250
pl_tex_destroy(gpu, &src_tex);
241-
pl_tex_destroy(gpu, &dst_tex);
251+
/* Keep dst_tex alive on the frame for the next placebo filter to
252+
* pick up via take_tex(). Unclaimed textures are freed automatically
253+
* by the frame destructor (frame_gpu_destroy). */
254+
placebo_frame_put_tex(frame, dst_tex, *image);
242255

243256
placebo_render_unlock();
244257

‎src/modules/placebo/filter_placebo_shader.c‎

Lines changed: 45 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434

3535
#ifdef _WIN32
3636
#include <windows.h>
37-
/* Use _stat on Windows to avoid deprecation warnings (S5) */
37+
/* MSVC marks stat() as deprecated; _stat is the replacement */
3838
#include <sys/stat.h>
3939
#define stat_func _stat
4040
#define stat_struct _stat
@@ -54,7 +54,8 @@ typedef struct
5454
char *loaded_text; /* inline text that was loaded */
5555
} shader_private;
5656

57-
/* C3 fix: single cleanup function used both by filter_close and mlt_properties destructor */
57+
/* Destructor registered via mlt_properties_set_data; handles both normal
58+
* filter teardown and early destruction if the filter is removed mid-session. */
5859
static void shader_private_destroy(void *ptr)
5960
{
6061
shader_private *priv = (shader_private *) ptr;
@@ -81,7 +82,7 @@ static char *read_file(const char *path, size_t *out_len)
8182
long len = ftell(f);
8283
fseek(f, 0, SEEK_SET);
8384

84-
if (len <= 0 || len > 10 * 1024 * 1024) { /* W7 fix: reject files > 10 MB */
85+
if (len <= 0 || len > 10 * 1024 * 1024) { /* sanity limit for shader files */
8586
fclose(f);
8687
return NULL;
8788
}
@@ -210,14 +211,14 @@ static int filter_get_image(mlt_frame frame,
210211
return mlt_frame_get_image(frame, image, format, width, height, writable);
211212
}
212213

213-
/* W2 fix: verify RGBA8 format is supported */
214+
/* Not all GPU backends guarantee rgba8 support (e.g. some OpenGL ES) */
214215
pl_fmt rgba_fmt = pl_find_named_fmt(gpu, "rgba8");
215216
if (!rgba_fmt) {
216217
mlt_log_error(MLT_FILTER_SERVICE(filter), "GPU does not support rgba8 format\n");
217218
return mlt_frame_get_image(frame, image, format, width, height, writable);
218219
}
219220

220-
/* Get or create private data (C3 fix: single destructor path) */
221+
/* Lazily allocate private data; destruction is handled by mlt_properties */
221222
shader_private *priv = mlt_properties_get_data(filter_props, "_shader_priv", NULL);
222223
if (!priv) {
223224
priv = calloc(1, sizeof(shader_private));
@@ -242,38 +243,48 @@ static int filter_get_image(mlt_frame frame,
242243
int h = *height;
243244
size_t stride = w * 4;
244245

245-
/* W4 fix: hold render lock for all GPU operations */
246+
/* pl_renderer is not thread-safe; hold the lock for the entire
247+
* upload → render → download sequence */
246248
placebo_render_lock();
247249

248-
/* Create source texture and upload */
249-
pl_tex src_tex = pl_tex_create(gpu,
250-
pl_tex_params(.w = w,
251-
.h = h,
252-
.format = rgba_fmt,
253-
.sampleable = true,
254-
.host_writable = true, ));
255-
if (!src_tex) {
256-
mlt_log_error(MLT_FILTER_SERVICE(filter), "Failed to create source texture\n");
257-
placebo_render_unlock();
258-
return 0;
259-
}
250+
/* Try to reuse a GPU texture left by a preceding placebo filter on
251+
* this frame. Returns NULL if there is none, or if the RAM buffer was
252+
* reallocated by an intervening CPU filter (stale texture). */
253+
pl_tex reused_src = placebo_frame_take_tex(frame, *image);
254+
pl_tex src_tex;
255+
if (reused_src) {
256+
src_tex = reused_src;
257+
} else {
258+
src_tex = pl_tex_create(gpu,
259+
pl_tex_params(.w = w,
260+
.h = h,
261+
.format = rgba_fmt,
262+
.sampleable = true,
263+
.host_writable = true, ));
264+
if (!src_tex) {
265+
mlt_log_error(MLT_FILTER_SERVICE(filter), "Failed to create source texture\n");
266+
placebo_render_unlock();
267+
return 0;
268+
}
260269

261-
/* C4 fix: check upload return */
262-
if (!pl_tex_upload(gpu,
263-
pl_tex_transfer_params(.tex = src_tex,
264-
.row_pitch = stride,
265-
.ptr = *image, ))) {
266-
mlt_log_error(MLT_FILTER_SERVICE(filter), "GPU texture upload failed\n");
267-
pl_tex_destroy(gpu, &src_tex);
268-
placebo_render_unlock();
269-
return 0;
270+
if (!pl_tex_upload(gpu,
271+
pl_tex_transfer_params(.tex = src_tex,
272+
.row_pitch = stride,
273+
.ptr = *image, ))) {
274+
mlt_log_error(MLT_FILTER_SERVICE(filter), "GPU texture upload failed\n");
275+
pl_tex_destroy(gpu, &src_tex);
276+
placebo_render_unlock();
277+
return 0;
278+
}
270279
}
271280

272-
/* Create destination texture */
281+
/* sampleable is needed so a subsequent placebo filter can bind this
282+
* texture as its source without re-uploading from RAM. */
273283
pl_tex dst_tex = pl_tex_create(gpu,
274284
pl_tex_params(.w = w,
275285
.h = h,
276286
.format = rgba_fmt,
287+
.sampleable = true,
277288
.renderable = true,
278289
.host_readable = true, ));
279290
if (!dst_tex) {
@@ -315,17 +326,20 @@ static int filter_get_image(mlt_frame frame,
315326
mlt_log_warning(MLT_FILTER_SERVICE(filter), "pl_render_image with shader hook failed\n");
316327
}
317328

318-
/* C4 fix: check download return */
329+
/* Always download to RAM — MLT expects *image to hold current pixels,
330+
* even though the texture may be reused on the GPU side. */
319331
if (!pl_tex_download(gpu,
320332
pl_tex_transfer_params(.tex = dst_tex,
321333
.row_pitch = stride,
322334
.ptr = *image, ))) {
323335
mlt_log_warning(MLT_FILTER_SERVICE(filter), "GPU texture download failed\n");
324336
}
325337

326-
/* Cleanup */
327338
pl_tex_destroy(gpu, &src_tex);
328-
pl_tex_destroy(gpu, &dst_tex);
339+
/* Keep dst_tex alive on the frame for the next placebo filter to
340+
* pick up via take_tex(). Unclaimed textures are freed automatically
341+
* by the frame destructor (frame_gpu_destroy). */
342+
placebo_frame_put_tex(frame, dst_tex, *image);
329343

330344
placebo_render_unlock();
331345

@@ -347,7 +361,6 @@ mlt_filter filter_placebo_shader_init(mlt_profile profile,
347361
mlt_filter filter = mlt_filter_new();
348362
if (filter) {
349363
filter->process = filter_process;
350-
/* C3 fix: no filter->close needed -- cleanup via mlt_properties destructor */
351364
mlt_properties props = MLT_FILTER_PROPERTIES(filter);
352365
mlt_properties_set(props, "shader_path", arg ? arg : "");
353366
mlt_properties_set(props, "shader_text", "");

‎src/modules/placebo/gpu_context.c‎

Lines changed: 79 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "gpu_context.h"
2121

22+
#include <framework/mlt_frame.h>
2223
#include <libplacebo/cache.h>
2324
#include <libplacebo/dispatch.h>
2425
#include <libplacebo/gpu.h>
@@ -63,7 +64,7 @@ static pl_d3d11 s_d3d11 = NULL;
6364

6465
#ifdef PL_HAVE_VULKAN
6566
static pl_vulkan s_vulkan = NULL;
66-
static pl_vk_inst s_vk_inst = NULL; /* stored to avoid leak (W1) */
67+
static pl_vk_inst s_vk_inst = NULL; /* must outlive pl_vulkan; destroyed in gpu_release */
6768
#ifdef _WIN32
6869
static HMODULE s_vulkan_dll = NULL; /* dynamically loaded vulkan-1.dll */
6970
#endif
@@ -80,7 +81,7 @@ static pl_cache s_cache = NULL;
8081

8182
static int s_initialized = 0;
8283

83-
/* ---------- Mutex (C1 fix: use SRWLOCK on Windows -- statically initializable) ---------- */
84+
/* ---------- Mutex (SRWLOCK on Windows is statically initializable, unlike CRITICAL_SECTION) ---------- */
8485

8586
#ifdef _WIN32
8687
static SRWLOCK s_mutex = SRWLOCK_INIT;
@@ -165,7 +166,7 @@ static void ensure_cache_dir(const char *path)
165166
char *last_sep = strrchr(dir, '/');
166167
if (last_sep) {
167168
*last_sep = '\0';
168-
mkdir(dir, 0755); /* W5 fix: actually create the directory */
169+
mkdir(dir, 0755);
169170
}
170171
#endif
171172
}
@@ -304,7 +305,6 @@ static int init_gpu(void)
304305

305306
load_cache();
306307

307-
/* C2 fix: register cleanup at process exit */
308308
atexit(placebo_gpu_release);
309309

310310
return 1;
@@ -358,7 +358,6 @@ pl_renderer placebo_renderer_get(void)
358358
return result;
359359
}
360360

361-
/* W4 fix: render lock for thread safety around pl_renderer/pl_dispatch calls */
362361
void placebo_render_lock(void)
363362
{
364363
RENDER_LOCK();
@@ -369,6 +368,81 @@ void placebo_render_unlock(void)
369368
RENDER_UNLOCK();
370369
}
371370

371+
/* ---------- Frame GPU texture reuse ----------
372+
*
373+
* When multiple placebo filters are chained on one producer, each filter
374+
* would normally upload the frame to GPU, render, and download back to RAM.
375+
* For N filters that means N uploads and N downloads — the intermediate
376+
* RAM roundtrips are pure waste because the next placebo filter will
377+
* re-upload the same data immediately.
378+
*
379+
* To avoid this, each placebo filter attaches its output texture to the
380+
* mlt_frame via put_tex(). The next placebo filter in the chain calls
381+
* take_tex() to grab it and use it directly as its source, skipping the
382+
* upload. The download to RAM still happens every time (MLT expects the
383+
* image buffer to be current), but the upload is eliminated for all
384+
* filters after the first one.
385+
*
386+
* Staleness detection: if a non-placebo CPU filter runs between two
387+
* placebo filters, it may reallocate the image buffer (e.g. via
388+
* mlt_frame_get_image with writable=1, which triggers a copy).
389+
* put_tex() records the buffer pointer, and take_tex() compares it
390+
* against the current pointer. A mismatch means the texture content
391+
* no longer matches RAM, so take_tex() returns NULL and the caller
392+
* falls back to a fresh upload. This is safe because MLT's standard
393+
* mechanism for a filter to modify frame data is to request a writable
394+
* buffer, which always produces a new allocation.
395+
*/
396+
397+
typedef struct
398+
{
399+
pl_tex tex;
400+
const uint8_t *image_ptr; /* RAM buffer address at time of put_tex */
401+
} placebo_frame_gpu;
402+
403+
/* Called by mlt_properties when the frame is destroyed or the property
404+
* is overwritten. Must acquire render_lock because pl_tex_destroy
405+
* touches GPU state, and this destructor may fire from any thread. */
406+
static void frame_gpu_destroy(void *ptr)
407+
{
408+
placebo_frame_gpu *d = ptr;
409+
if (d && d->tex) {
410+
pl_gpu gpu = placebo_gpu_get();
411+
if (gpu) {
412+
placebo_render_lock();
413+
pl_tex_destroy(gpu, &d->tex);
414+
placebo_render_unlock();
415+
}
416+
}
417+
free(d);
418+
}
419+
420+
pl_tex placebo_frame_take_tex(mlt_frame frame, const uint8_t *current_image)
421+
{
422+
mlt_properties props = MLT_FRAME_PROPERTIES(frame);
423+
placebo_frame_gpu *d = mlt_properties_get_data(props, "_placebo_gpu", NULL);
424+
if (!d || !d->tex)
425+
return NULL;
426+
if (d->image_ptr != current_image)
427+
return NULL; /* buffer was reallocated — texture is stale */
428+
pl_tex tex = d->tex;
429+
d->tex = NULL; /* transfer ownership to caller; disarm destructor */
430+
return tex;
431+
}
432+
433+
void placebo_frame_put_tex(mlt_frame frame, pl_tex tex, const uint8_t *image_ptr)
434+
{
435+
placebo_frame_gpu *d = calloc(1, sizeof(placebo_frame_gpu));
436+
d->tex = tex;
437+
d->image_ptr = image_ptr;
438+
/* If a previous texture was attached, set_data replaces it and
439+
* frame_gpu_destroy fires for the old one. After take_tex() the
440+
* old entry has tex=NULL so the destructor becomes a no-op. */
441+
mlt_properties_set_data(MLT_FRAME_PROPERTIES(frame), "_placebo_gpu", d, 0, frame_gpu_destroy, NULL);
442+
}
443+
444+
/* ---------- GPU teardown ---------- */
445+
372446
void placebo_gpu_release(void)
373447
{
374448
LOCK();
@@ -405,7 +479,6 @@ void placebo_gpu_release(void)
405479
pl_vulkan_destroy(&s_vulkan);
406480
s_vulkan = NULL;
407481
}
408-
/* W1 fix: destroy Vulkan instance */
409482
if (s_vk_inst) {
410483
pl_vk_inst_destroy(&s_vk_inst);
411484
s_vk_inst = NULL;

0 commit comments

Comments
 (0)