From 0b217575d83c2221ae019c4102c4e1b265a02c57 Mon Sep 17 00:00:00 2001 From: chrg6 Date: Fri, 13 Nov 2026 17:12:08 -0500 Subject: [PATCH] Keep the movie deband planes per decoder, and drop the frame instead of ending the game when they cannot be allocated. make_picture allocated the three 16-bit deband planes (6.2 MB at 1080p) for every picture and freed them after it, 60 times a second while a movie played, and in its usual path (8-bit, deband on, no overlay) a failed allocation ended the game with "movie: out of memory for a WxH frame", while the slower path and the frame allocation itself drop the frame through movie_oom_log. run_stripes' comment records that FATAL at 45.7 GB of commit on an RX 580. A verifier's probe measured 0.34-0.36 ms and about 1000 page faults a picture for the per-picture allocation against 0.04 ms and 3 faults with kept planes, about 12% of the convert thread's time per picture. Each Video now keeps its planes (y16, u16, v16, plane_room), used only by its convert thread and made again only for a larger picture; planes_for frees what it has and allocates afresh, and on a failure frees all three and keeps none. Both paths take their planes from it and drop the frame through movie_oom_log when it fails. finalize frees the planes after stop_thread has joined the convert thread, which also covers the destructor. The path with the deband off and no overlay no longer allocates planes it never used. tests/movie_planes_check.py cuts the deband, the conversions, run_stripes, planes_for and make_picture from ov_movie2.c with a stub dav1d picture and a malloc that fails on request: two pictures of one size allocate the planes once, a larger one makes them again, the NV12 frame (and the overlay's RGBA frame) equals what fresh planes give, also with an earlier picture left in the kept planes, and a failed plane allocation drops the frame with nothing left allocated. The compile check passes with no new warnings. Co-Authored-By: Claude Opus 5.5 --- runtime/ov_movie2.c | 55 +++++++--- tests/movie_planes_check.py | 212 ++++++++++++++++++++++++++++++++++++ 2 files changed, 252 insertions(+), 15 deletions(-) create mode 100644 tests/movie_planes_check.py diff --git a/runtime/ov_movie2.c b/runtime/ov_movie2.c index 4b89087c..1ca0de30 100644 --- a/runtime/ov_movie2.c +++ b/runtime/ov_movie2.c @@ -63,6 +63,11 @@ typedef struct { PlatCond conv; // a decoded picture arrived, a slot freed, or quit Decoded cq[CQMAX]; int cqh, cqn; + // The convert thread's deband planes (16-bit Y, U and V, plane_room samples of luma), kept from picture to picture + // and made again only for a larger one: made per picture they were 6.2 MB of fresh pages each at 1080p, 60 times a + // second. finalize frees them once the convert thread is joined. + uint16_t *y16, *u16, *v16; + size_t plane_room; Dav1dContext *dav1d; uint8_t in_ev[0x40], out_ev[0x40]; // nn::os::SystemEventType each int in_bound, out_bound; @@ -357,8 +362,23 @@ static void movie_oom_log(int w, int h) { if (plat_xchg32(&drops, 1) == 0) rt_log("movie: out of memory for a %dx%d frame: dropped (later drops not logged)", w, h); } -// One dav1d picture (8- or 10-bit 4:2:0) -> NV12 with 8-bit samples, and its RGBA; decoder thread -static int make_picture(const Dav1dPicture *pic, Picture *p) { +// The Video's deband planes for a w x h picture (convert thread). 0: no memory for them; the frame is then dropped +// as for any allocation of a picture, and no planes are kept. +static int planes_for(Video *v, int w, int h) { + size_t px = (size_t)w * h; + if (px <= v->plane_room) return 1; + free(v->y16), free(v->u16), free(v->v16); + v->y16 = malloc(px * 2), v->u16 = malloc(px / 2), v->v16 = malloc(px / 2); + v->plane_room = px; + if (v->y16 && v->u16 && v->v16) return 1; + free(v->y16), free(v->u16), free(v->v16); + v->y16 = v->u16 = v->v16 = NULL; + v->plane_room = 0; + return 0; +} + +// One dav1d picture (8- or 10-bit 4:2:0) -> NV12 with 8-bit samples, and its RGBA; convert thread +static int make_picture(Video *v, const Dav1dPicture *pic, Picture *p) { int w = pic->p.w, h = pic->p.h, hbd = pic->p.bpc > 8, sh = pic->p.bpc - 8; if (pic->p.layout != DAV1D_PIXEL_LAYOUT_I420) { static volatile int32_t logged; @@ -395,11 +415,14 @@ static int make_picture(const Dav1dPicture *pic, Picture *p) { // planes and writes the dithered NV12 rows. Reading the decoder's planes instead of an NV12 copy of them gives // the same samples, and nothing reads the frame while it is written, so each stripe finishes its rows alone: // one round of threads per frame instead of two, and no single-threaded copy before it. - uint16_t *Y16 = malloc((size_t)w * h * 2), *U16 = malloc((size_t)w * h / 2), *V16 = malloc((size_t)w * h / 2); - if (!Y16 || !U16 || !V16) rt_fatal("movie: out of memory for a %dx%d frame", w, h); - Stripe base = {2, 0, 0, w, h, o, Y16, U16, V16, NULL, pic->data[0], pic->data[1], pic->data[2], ys, cs}; + if (!planes_for(v, w, h)) { + free(o); + movie_oom_log(w, h); + return 0; + } + Stripe base = {2, 0, 0, w, h, o, v->y16, v->u16, v->v16, NULL, pic->data[0], pic->data[1], pic->data[2], + ys, cs}; run_stripes(base); - free(Y16), free(U16), free(V16); *p = (Picture){o, NULL, w, h, pic->m.timestamp, 0}; return 1; } @@ -417,27 +440,26 @@ static int make_picture(const Dav1dPicture *pic, Picture *p) { } } // debanded planes (NX2NAVI_MOVIE_DEBAND=0 keeps the decoded ones), then the overlay's RGBA or the dithered NV12 + if (!deband_on && !rgba) { + *p = (Picture){o, NULL, w, h, pic->m.timestamp, 0}; + return 1; + } { - uint16_t *Y16 = malloc((size_t)w * h * 2), *U16 = malloc((size_t)w * h / 2), *V16 = malloc((size_t)w * h / 2); - if (!Y16 || !U16 || !V16) { - free(Y16), free(U16), free(V16), free(o), free(rgba); + if (!planes_for(v, w, h)) { + free(o), free(rgba); movie_oom_log(w, h); return 0; } + uint16_t *Y16 = v->y16, *U16 = v->u16, *V16 = v->v16; Stripe base = {0, 0, 0, w, h, o, Y16, U16, V16, rgba}; if (deband_on) { run_stripes(base); - } else if (!rgba) { - free(Y16), free(U16), free(V16); - *p = (Picture){o, rgba, w, h, pic->m.timestamp, 0}; - return 1; } else { for (size_t i = 0; i < (size_t)w * h; i++) Y16[i] = (uint16_t)(o[i] * 16); for (size_t i = 0; i < (size_t)w * h / 4; i++) U16[i] = (uint16_t)(uv[2 * i] * 16), V16[i] = (uint16_t)(uv[2 * i + 1] * 16); } base.phase = 1; run_stripes(base); - free(Y16), free(U16), free(V16); } *p = (Picture){o, rgba, w, h, pic->m.timestamp, 0}; return 1; @@ -505,7 +527,7 @@ static void convert_thread(void *arg) { } else { uint64_t t0 = plat_now_ns(); Picture p; - int ok = make_picture(&d.pic, &p); + int ok = make_picture(v, &d.pic, &p); t_make_ns += plat_now_ns() - t0; n_made++; dav1d_picture_unref(&d.pic); @@ -600,6 +622,9 @@ static void stop_thread(Video *v) { static void finalize(Video *v) { stop_thread(v); + free(v->y16), free(v->u16), free(v->v16); // the convert thread, their only user, has been joined + v->y16 = v->u16 = v->v16 = NULL; + v->plane_room = 0; plat_mutex_lock(&v->codec); if (v->dav1d) dav1d_close(&v->dav1d); plat_mutex_unlock(&v->codec); diff --git a/tests/movie_planes_check.py b/tests/movie_planes_check.py new file mode 100644 index 00000000..f8d9774d --- /dev/null +++ b/tests/movie_planes_check.py @@ -0,0 +1,212 @@ +"""Movie conversion keeps its deband planes per decoder and drops a frame it has no memory for +(runtime/ov_movie2.c planes_for, make_picture): builds the deband, the conversions, run_stripes, planes_for and +make_picture, cut from ov_movie2.c by their text, with stand-ins for dav1d's picture, the platform calls and a malloc +that fails on request, and checks that + + - two pictures of one size allocate the planes once, a larger picture makes them again; + - the NV12 frame (and with the overlay on, the RGBA frame) equals what fresh planes give for the same input, also + when the kept planes still hold an earlier picture; + - a plane allocation that fails drops the frame (0) without a fatal and leaves nothing allocated, and the next + picture gets its planes. + +No game files, no dav1d. Usage: python tests/movie_planes_check.py [--cc ] +""" +import argparse +import os +import subprocess +import sys +import tempfile +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +MSYS_BIN = Path(r'C:\msys64\ucrt64\bin') + +HARNESS = r''' +#include +#include +#include +#include +#include +enum { DAV1D_PIXEL_LAYOUT_I420 = 1 }; +typedef struct { int w, h, bpc, layout; } Dav1dPictureParameters; +typedef struct { int color_range, mtrx, pri, trc; } Dav1dSequenceHeader; +typedef struct { int64_t timestamp; } Dav1dDataProps; +typedef struct { + Dav1dPictureParameters p; + const Dav1dSequenceHeader *seq_hdr; + void *data[3]; + ptrdiff_t stride[2]; + Dav1dDataProps m; +} Dav1dPicture; +typedef struct { uint8_t *nv12, *rgba; int w, h; int64_t ts; int eos; } Picture; +typedef struct { uint16_t *y16, *u16, *v16; size_t plane_room; } Video; +typedef int PlatMutex; +typedef int PlatCond; +#define PLAT_MUTEX_INIT 0 +#define PLAT_COND_INIT 0 +static void plat_mutex_lock(PlatMutex *m) { (void)m; } +static void plat_mutex_unlock(PlatMutex *m) { (void)m; } +static void plat_cond_wait(PlatCond *c, PlatMutex *m) { (void)c, (void)m; } +static void plat_cond_broadcast(PlatCond *c) { (void)c; } +static int plat_thread_spawn(void (*fn)(void *), void *arg, const char *name, int prio) { + (void)fn, (void)arg, (void)name, (void)prio; + return 0; // no helper threads: run_stripes runs every stripe itself +} +static int32_t plat_xchg32(volatile int32_t *p, int32_t v) { return __atomic_exchange_n(p, v, __ATOMIC_SEQ_CST); } +static void rt_log(const char *fmt, ...) { (void)fmt; } +static void rt_fatal(const char *fmt, ...) { printf("FATAL: %%s\n", fmt); fflush(stdout); exit(3); } +static int overlay_on; +static const char *cfg_get(const char *name) { return !strcmp(name, "NX2NAVI_MOVIE_OVERLAY") && overlay_on ? "1" : NULL; } +// Counted allocations; fail_at: the call number that returns NULL (0: none). +static int calls, live, fail_at; +static void *t_malloc(size_t n) { + if (++calls == fail_at) return NULL; + void *p = malloc(n); + if (p) live++; + return p; +} +static void t_free(void *p) { + if (p) live--; + free(p); +} +#define malloc t_malloc +#define free t_free +%s +#undef malloc +#undef free +static int fails; +#define CHECK(cond, ...) do { if (!(cond)) { printf("FAIL: " __VA_ARGS__); putchar('\n'); fails++; } } while (0) + +// An 8-bit 4:2:0 picture: smooth gradients with a little texture, in planes wider than the picture (dav1d's strides). +static void picture(Dav1dPicture *pic, int w, int h, int seed, uint8_t *planes) { + memset(pic, 0, sizeof *pic); + pic->p = (Dav1dPictureParameters){w, h, 8, DAV1D_PIXEL_LAYOUT_I420}; + pic->stride[0] = w + 32, pic->stride[1] = w / 2 + 16; + uint8_t *y = planes, *u = y + pic->stride[0] * h, *v = u + pic->stride[1] * (h / 2); + for (int r = 0; r < h; r++) + for (int x = 0; x < w; x++) y[r * pic->stride[0] + x] = (uint8_t)(16 + (x + r + seed) / 3 + ((x * r + seed) %% 5 == 0)); + for (int r = 0; r < h / 2; r++) + for (int x = 0; x < w / 2; x++) { + u[r * pic->stride[1] + x] = (uint8_t)(100 + (x + seed) / 4); + v[r * pic->stride[1] + x] = (uint8_t)(140 - (r + seed) / 4); + } + pic->data[0] = y, pic->data[1] = u, pic->data[2] = v; + pic->m.timestamp = seed; +} + +// What fresh planes give: the NV12 frame (and the RGBA one with the overlay on) from one stripe over every row. +static void reference(const Dav1dPicture *pic, uint8_t *nv12, uint8_t *rgba) { + int w = pic->p.w, h = pic->p.h; + uint16_t *Y = malloc((size_t)w * h * 2), *U = malloc((size_t)w * h / 2), *V = malloc((size_t)w * h / 2); + if (!overlay_on) { + Stripe s = {2, 0, h, w, h, nv12, Y, U, V, NULL, pic->data[0], pic->data[1], pic->data[2], pic->stride[0], + pic->stride[1]}; + stripe_run(&s); + } else { + for (int r = 0; r < h; r++) memcpy(nv12 + (size_t)r * w, (uint8_t *)pic->data[0] + r * pic->stride[0], (size_t)w); + uint8_t *uv = nv12 + (size_t)w * h; + for (int r = 0; r < h / 2; r++) + for (int x = 0; x < w / 2; x++) { + uv[(size_t)r * w + 2 * x] = ((uint8_t *)pic->data[1])[r * pic->stride[1] + x]; + uv[(size_t)r * w + 2 * x + 1] = ((uint8_t *)pic->data[2])[r * pic->stride[1] + x]; + } + Stripe s = {0, 0, h, w, h, nv12, Y, U, V, rgba}; + stripe_run(&s); + s.phase = 1; + stripe_run(&s); + } + free(Y), free(U), free(V); +} + +static void same(const Picture *p, const Dav1dPicture *pic, const char *what) { + int w = pic->p.w, h = pic->p.h; + uint8_t *nv12 = calloc((size_t)w * h * 3 / 2, 1), *rgba = calloc((size_t)w * h * 4, 1); + reference(pic, nv12, rgba); + if (!overlay_on) CHECK(!memcmp(p->nv12, nv12, (size_t)w * h * 3 / 2), "%%s: the NV12 frame equals fresh planes'", what); + else CHECK(p->rgba && !memcmp(p->rgba, rgba, (size_t)w * h * 4), "%%s: the RGBA frame equals fresh planes'", what); + free(nv12), free(rgba); +} + +static void release(Picture *p) { + t_free(p->nv12), t_free(p->rgba); + p->nv12 = p->rgba = NULL; +} + +int main(int argc, char **argv) { + overlay_on = argc > 1 && !strcmp(argv[1], "overlay"); + const char *mode = overlay_on ? "overlay" : "nv12"; + uint8_t *planes = malloc(1 << 20); + Dav1dPicture pic; + Picture p; + Video v = {0}; + int per_pic = overlay_on ? 2 : 1; // the frame allocations of a picture besides the planes + picture(&pic, 64, 48, 1, planes); + CHECK(make_picture(&v, &pic, &p) == 1, "%%s: the first picture is made", mode); + CHECK(calls == per_pic + 3 && v.plane_room == 64 * 48, "%%s: the first picture allocates the planes (%%d calls)", mode, calls); + same(&p, &pic, "first picture"); + release(&p); + picture(&pic, 64, 48, 7, planes); // other content, same size: the planes hold the first picture's values + CHECK(make_picture(&v, &pic, &p) == 1, "%%s: the second picture is made", mode); + CHECK(calls == 2 * per_pic + 3, "%%s: the second picture of that size reuses the planes (%%d calls)", mode, calls); + same(&p, &pic, "second picture"); + release(&p); + CHECK(live == 3, "%%s: only the three planes stay allocated between pictures (%%d)", mode, live); + picture(&pic, 96, 64, 3, planes); + CHECK(make_picture(&v, &pic, &p) == 1 && v.plane_room == 96 * 64, "%%s: a larger picture is made", mode); + CHECK(calls == 3 * per_pic + 6 && live == 3 + per_pic, "%%s: it makes the planes again (%%d calls, %%d live)", mode, calls, live); + same(&p, &pic, "larger picture"); + release(&p); + picture(&pic, 64, 48, 5, planes); + CHECK(make_picture(&v, &pic, &p) == 1 && v.plane_room == 96 * 64, "%%s: a smaller picture uses the larger planes", mode); + same(&p, &pic, "smaller picture"); + release(&p); + t_free(v.y16), t_free(v.u16), t_free(v.v16); // finalize + // A second decoder whose U plane allocation fails: the frame is dropped, nothing stays allocated. + Video w2 = {0}; + int before = live; + fail_at = calls + per_pic + 2; + CHECK(make_picture(&w2, &pic, &p) == 0, "%%s: a failed plane allocation drops the frame", mode); + CHECK(live == before && !w2.y16 && !w2.u16 && !w2.v16 && !w2.plane_room, + "%%s: nothing stays allocated after the failure (%%d live, %%d before)", mode, live, before); + fail_at = 0; + CHECK(make_picture(&w2, &pic, &p) == 1 && w2.plane_room == 64 * 48, "%%s: the next picture gets its planes", mode); + same(&p, &pic, "picture after a failure"); + release(&p); + t_free(w2.y16), t_free(w2.u16), t_free(w2.v16); + CHECK(live == 0, "%%s: everything freed (%%d live)", mode, live); + printf(fails ? "movie planes (%%s): %%d failures\n" : "movie planes (%%s): ok\n", mode, fails); + return fails != 0; +} +''' + + +def cut(text, start, end_marker): + a = text.index(start) + b = text.index(end_marker, a) + return text[a:b] + + +def main(): + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument('--cc', help='the C compiler (default: MSYS2 UCRT64 clang on Windows, clang elsewhere)') + args = ap.parse_args() + win = os.name == 'nt' + cc = args.cc or (str(MSYS_BIN / 'clang.exe') if win and MSYS_BIN.exists() else 'clang') + env = dict(os.environ) + if win and MSYS_BIN.exists(): + env['PATH'] = str(MSYS_BIN) + os.pathsep + env.get('PATH', '') + src = (ROOT / 'runtime' / 'ov_movie2.c').read_text(encoding='utf-8') + body = cut(src, '#define DEBAND_THRESH', '\n// Queues a decoded picture, waiting while the queue is full') + with tempfile.TemporaryDirectory(prefix='nx2navi-movie-planes-') as tmp: + c = Path(tmp) / 'check.c' + c.write_text(HARNESS % body, encoding='utf-8') + exe = Path(tmp) / ('check.exe' if win else 'check') + subprocess.check_call([cc, '-O2', '-Wall', '-Wno-unused-function', str(c), '-o', str(exe), '-lm'], env=env) + rc = 0 + for mode in ([], ['overlay']): + rc = subprocess.call([str(exe), *mode]) or rc + return rc + + +if __name__ == '__main__': + sys.exit(main())