mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-09 03:29:43 -10:00
video_thread: a duped frame reaches the driver as NULL, not as an old frame
With threaded video on, the ffmpeg core ghosted on Vulkan and D3D12 - every frame doubled with an older one - and on D3D11 faded to grey and back. gl and glcore were clean, because there the core renders with OpenGL itself and never sends pixels at all. A core with nothing new calls video_refresh with NULL, and the driver repeats the frame it has. The ffmpeg core does that for every display refresh that falls between two video frames: every other call for a 30 fps clip on a 60 Hz display. video_thread_frame() treated that push like any other. It picked a free ring slot, copied nothing into it because there was nothing to copy, and published it. The video thread then gave the driver that slot's buffer as the frame. What the buffer held was whatever had last been put there: the frame from two pushes before, or - when the real frames all arrive zero-copy in the other slot, as the ffmpeg core's now do - something older still, down to the 0x80 fill the slots are allocated with. The driver uploaded it and showed it as new. So the display went new, old, new, old at the refresh rate. A slot now records that its push carried no pixels, and for such a slot the video thread hands the driver NULL, which is what the core sent and what every driver already gets without the wrapper. The pixel-format conversion and the video filter are skipped for it, as they have nothing to work on. A hardware frame the ring could not publish takes the same route; the comment there already said it was to be treated as a dupe. Regression test: a new lane in the threaded_video harness pushes pixels, NULL, pixels, NULL through the wrapper with the driver's frame() interposed, and requires the driver to see exactly that. On the old code it fails with the driver given stale pixels for every dupe; with this change it passes. Also run on a real build under Xvfb, ffmpeg core on the software path: Vulkan and gl1, threaded video on and off.
This commit is contained in:
1 parent
a38053c8f8
commit
8ae3e66882
3 files changed
+127
-3
No files matched your search
@@ -1778,15 +1778,18 @@ static void video_thread_loop(void *data)
|
||||
}
|
||||
else
|
||||
{
|
||||
const void *fdata = thr->frame.slot[slot].buffer;
|
||||
/* A dupe goes to the driver as the NULL the core
|
||||
* sent, exactly as it does without the wrapper. */
|
||||
const void *fdata = thr->frame.slot[slot].dupe
|
||||
? NULL : thr->frame.slot[slot].buffer;
|
||||
unsigned fwidth = thr->frame.slot[slot].width;
|
||||
unsigned fheight = thr->frame.slot[slot].height;
|
||||
unsigned fpitch = thr->frame.slot[slot].pitch;
|
||||
if (thr->frame.slot[slot].convert)
|
||||
if (fdata && thr->frame.slot[slot].convert)
|
||||
video_thread_convert(thr, thr->frame.slot[slot].convert,
|
||||
&fdata, fwidth, fheight, &fpitch);
|
||||
#ifdef HAVE_VIDEO_FILTER
|
||||
if (thr->frame.slot[slot].filter_bpp)
|
||||
if (fdata && thr->frame.slot[slot].filter_bpp)
|
||||
video_thread_filter(thr, &fdata, &fwidth, &fheight, &fpitch);
|
||||
#endif
|
||||
ret = thr->driver->frame(thr->driver_data,
|
||||
@@ -2468,6 +2471,11 @@ static bool video_thread_frame(void *data, const void *frame_,
|
||||
thr->frame.slot[slot].count = frame_count;
|
||||
thr->frame.slot[slot].pushed_at = now;
|
||||
thr->frame.slot[slot].hw_slot = hw_slot;
|
||||
/* Nothing was put in the slot: not a lent slot the core filled,
|
||||
* not a copy, not a hardware frame. What the buffer holds is the
|
||||
* frame from some earlier push, and must not be shown as this
|
||||
* one. */
|
||||
thr->frame.slot[slot].dupe = !zero_copy && !src && hw_slot < 0;
|
||||
/* Textures released since the last handoff ride with this frame */
|
||||
if (thr->tex_retire)
|
||||
{
|
||||
|
||||
@@ -505,6 +505,10 @@ typedef struct thread_video
|
||||
/* Hardware-rendered frame: the HW ring slot it lives in, -1
|
||||
* for a software frame. See hw_ring below. */
|
||||
int hw_slot;
|
||||
/* The push carried no pixels (the core duped): the driver is
|
||||
* given NULL and repeats what it has, never this buffer -
|
||||
* which holds whatever frame was last put in it, an old one. */
|
||||
bool dupe;
|
||||
uint8_t *buffer;
|
||||
unsigned width;
|
||||
unsigned height;
|
||||
|
||||
@@ -1395,6 +1395,117 @@ static void lane_resize_under_wrapper(void)
|
||||
rslane_seen_w, rslane_seen_h);
|
||||
}
|
||||
|
||||
/* A duped frame under the wrapper.
|
||||
*
|
||||
* A core that has nothing new calls video_refresh with NULL, and the
|
||||
* driver repeats what it has. The wrapper's push picked a ring slot for
|
||||
* that push like any other, put nothing in it, and the video thread
|
||||
* then handed the driver the slot's buffer as the frame: whatever frame
|
||||
* had last been copied there, two or more pushes old, or the 0x80 the
|
||||
* slots are born with. A core that dupes every other frame - the ffmpeg
|
||||
* core playing 30 fps at 60 Hz does - alternated each new frame with an
|
||||
* old one: ghosting on Vulkan and D3D12, fades to grey on D3D11.
|
||||
*
|
||||
* So: real frame, dupe, real frame, dupe, and the driver must see
|
||||
* exactly that - pixels, NULL, pixels, NULL. */
|
||||
#define DUPLANE_PUSHES 6
|
||||
static video_driver_t duplane_driver;
|
||||
static const video_driver_t *duplane_inner;
|
||||
static int duplane_seen[DUPLANE_PUSHES * 4];
|
||||
static retro_atomic_size_t duplane_count;
|
||||
static retro_atomic_size_t duplane_on;
|
||||
|
||||
static bool duplane_frame(void *data, const void *frame,
|
||||
unsigned width, unsigned height, uint64_t frame_count,
|
||||
unsigned pitch, const char *msg, video_frame_info_t *video_info)
|
||||
{
|
||||
if (retro_atomic_load_acquire_size(&duplane_on))
|
||||
{
|
||||
size_t n = retro_atomic_load_acquire_size(&duplane_count);
|
||||
if (n < sizeof(duplane_seen) / sizeof(duplane_seen[0]))
|
||||
{
|
||||
duplane_seen[n] = frame ? (int)*(const uint8_t*)frame : -1;
|
||||
retro_atomic_store_release_size(&duplane_count, n + 1);
|
||||
}
|
||||
}
|
||||
/* The inner driver is not given the test pattern to draw. */
|
||||
return duplane_inner->frame(data, NULL, width, height, frame_count,
|
||||
pitch, msg, video_info);
|
||||
}
|
||||
|
||||
static void lane_dupe_under_wrapper(void)
|
||||
{
|
||||
unsigned had = failures;
|
||||
unsigned i;
|
||||
size_t seen;
|
||||
thread_video_t *thr;
|
||||
uint8_t *pix;
|
||||
const unsigned w = 32, h = 32;
|
||||
|
||||
set_threaded_via_setting(true);
|
||||
run_frames(3);
|
||||
expect_wrapper(true, "dupe lane");
|
||||
thr = (thread_video_t*)video_state_get_ptr()->data;
|
||||
if (menu_is_up())
|
||||
command_event(CMD_EVENT_MENU_TOGGLE, NULL);
|
||||
CHECK(!menu_is_up(), "dupe lane: menu still up");
|
||||
run_frames(2);
|
||||
video_thread_wait_idle();
|
||||
|
||||
pix = (uint8_t*)malloc(w * h * sizeof(uint32_t));
|
||||
CHECK(pix != NULL, "dupe lane: no memory");
|
||||
if (!pix)
|
||||
return;
|
||||
|
||||
duplane_inner = thr->driver;
|
||||
duplane_driver = *thr->driver;
|
||||
duplane_driver.frame = duplane_frame;
|
||||
thr->driver = &duplane_driver;
|
||||
retro_atomic_store_release_size(&duplane_count, 0);
|
||||
retro_atomic_store_release_size(&duplane_on, 1);
|
||||
|
||||
for (i = 0; i < DUPLANE_PUSHES; i++)
|
||||
{
|
||||
if (i & 1)
|
||||
video_driver_frame(NULL, w, h, w * sizeof(uint32_t));
|
||||
else
|
||||
{
|
||||
/* 0x10, 0x20, 0x30: never 0x80, never each other. */
|
||||
memset(pix, 0x10 * (int)(i / 2 + 1), w * h * sizeof(uint32_t));
|
||||
video_driver_frame(pix, w, h, w * sizeof(uint32_t));
|
||||
}
|
||||
/* One at a time, so no push replaces another in the ring. */
|
||||
video_thread_wait_idle();
|
||||
}
|
||||
|
||||
retro_atomic_store_release_size(&duplane_on, 0);
|
||||
video_thread_wait_idle();
|
||||
thr->driver = duplane_inner;
|
||||
|
||||
seen = retro_atomic_load_acquire_size(&duplane_count);
|
||||
CHECK(seen == DUPLANE_PUSHES,
|
||||
"dupe lane: %u pushes reached the driver as %u frames",
|
||||
(unsigned)DUPLANE_PUSHES, (unsigned)seen);
|
||||
for (i = 0; i < DUPLANE_PUSHES && i < seen; i++)
|
||||
{
|
||||
if (i & 1)
|
||||
CHECK(duplane_seen[i] == -1,
|
||||
"push %u was a dupe (NULL) and the driver was given pixels (0x%02x): "
|
||||
"a stale ring slot shown as a new frame", i, duplane_seen[i]);
|
||||
else
|
||||
CHECK(duplane_seen[i] == 0x10 * (int)(i / 2 + 1),
|
||||
"push %u: the driver saw 0x%02x, the core sent 0x%02x",
|
||||
i, duplane_seen[i], 0x10 * (int)(i / 2 + 1));
|
||||
}
|
||||
|
||||
free(pix);
|
||||
if (!menu_is_up())
|
||||
command_event(CMD_EVENT_MENU_TOGGLE, NULL);
|
||||
run_frames(2);
|
||||
if (failures == had)
|
||||
fprintf(stderr, "[pass] dupe lane (NULL reaches the driver as NULL)\n");
|
||||
}
|
||||
|
||||
/* Heap traffic on the frame path.
|
||||
*
|
||||
* The emulation frame path makes no general-purpose heap calls today -
|
||||
@@ -2278,6 +2389,7 @@ int main(int argc, char *argv[])
|
||||
lane_waiter_call();
|
||||
lane_frame_path_heap();
|
||||
lane_resize_under_wrapper();
|
||||
lane_dupe_under_wrapper();
|
||||
}
|
||||
else
|
||||
fprintf(stderr, "[skip] null-driver instrumented lanes (real driver: %s)\n",
|
||||
|
||||
Reference in new issue
Block a user