Skip to content

Commit 63f184f

Browse files
committed
menu: pump a pending playlist read every frame, so it finishes on its own
Third report from this path: opening a playlist showed "No Playlist Entries Available", and backing out and opening the same playlist again showed the entries. Sometimes back did not work on the empty screen either. The missing piece was a pump. ENTRIES_NEED_REFRESH is consumed in exactly one place - generic_menu_entry_action() - which runs when the user presses something. There is no per-frame equivalent, so a displaylist that yielded mid-build stayed unfinished until the next keypress: the placeholder was drawn, nothing advanced the read, and the list only filled when an action happened to drive the rebuild. Re-entering the playlist worked because that WAS the input. menu_driver_iterate() now advances a pending read, once per frame, under the same shared I/O window the directory walks and the scanner use, and rebuilds the list itself the moment the read completes - without waiting for input. With nothing outstanding it costs one branch, so the ordinary case is unchanged. This is only reachable at Android/SAF speed. With local stdio a playlist is read inside the first slice and the yield path is never taken, which is why none of these three reports reproduced on a desktop. The oracle gains a pump lane that runs over the short-read VFS standing in for SAF: it asserts the first slice really does yield - so the lane cannot stop covering its own case - and then advances only by repeated frames, no requests and no input, until the read completes and the requested playlist is installed with its own 600 entries. Removing the advance leaves it pending forever, which is the reported symptom. Verified: strict C89 gates on playlist.c and menu_driver.c; full Linux build links with HAVE_QT=1; the playlist oracle passes plain, twice under ASan+UBSan+LSan and twice under TSan; the playlist manager oracle still passes. Still not exercised here: the menu-side rebuild itself. The lane drives the continue/finish pair the pump calls, not menu_driver_iterate(), because nothing links the real menu yet. A harness that links the shipping objects with only main replaced is the missing piece for that, and this path has now earned it.
1 parent 5f586c6 commit 63f184f

4 files changed

Lines changed: 156 additions & 0 deletions

File tree

menu/menu_driver.c

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2472,6 +2472,11 @@ static bool menu_driver_displaylist_push_internal(
24722472
return false;
24732473
}
24742474

2475+
static bool menu_playlist_within_budget(void *ud)
2476+
{
2477+
return task_nbio_slice_within_budget(ud, 0, 0);
2478+
}
2479+
24752480
static bool menu_driver_displaylist_push(
24762481
struct menu_state *menu_st,
24772482
settings_t *settings,
@@ -8202,6 +8207,57 @@ int generic_menu_entry_action(
82028207
}
82038208

82048209
/* Iterate the menu driver for one frame. */
8210+
/* Advance work a displaylist left unfinished, and rebuild the list
8211+
* when it completes.
8212+
*
8213+
* ENTRIES_NEED_REFRESH is otherwise consumed only in
8214+
* generic_menu_entry_action(), i.e. when the user presses something.
8215+
* Anything that yields mid-build therefore sat unfinished until the
8216+
* next keypress: a playlist read that did not fit its first slice
8217+
* showed an empty list, and populated only after backing out and
8218+
* re-entering. This is the pump that was missing - it runs every
8219+
* frame, with no input required.
8220+
*
8221+
* Only a genuinely pending parse does any work here, so a menu with
8222+
* nothing outstanding costs one predictable branch. */
8223+
static void menu_driver_pump_pending(struct menu_state *menu_st,
8224+
settings_t *settings)
8225+
{
8226+
nbio_budget_t b;
8227+
menu_list_t *menu_list;
8228+
file_list_t *selection_buf;
8229+
file_list_t *menu_stack;
8230+
int r;
8231+
8232+
if (!playlist_init_cached_pending())
8233+
return;
8234+
8235+
/* One slice of the shared per-frame I/O window, the same budget
8236+
* the directory walks and the scanner draw from. */
8237+
task_nbio_slice_open(&b);
8238+
r = playlist_init_cached_continue(menu_playlist_within_budget, &b);
8239+
task_nbio_slice_close(&b);
8240+
8241+
if (r == 0)
8242+
return; /* still reading; come back next frame */
8243+
8244+
if (r > 0)
8245+
playlist_init_cached_finish();
8246+
8247+
/* Ready (or failed): rebuild the list now rather than waiting for
8248+
* the user to press something. */
8249+
if (!(menu_list = menu_st->entries.list))
8250+
return;
8251+
selection_buf = MENU_LIST_GET_SELECTION(menu_list, 0);
8252+
menu_stack = MENU_LIST_GET(menu_list, 0);
8253+
8254+
if (selection_buf && menu_stack)
8255+
menu_driver_displaylist_push(menu_st, settings,
8256+
selection_buf, menu_stack);
8257+
8258+
menu_st->flags &= ~MENU_ST_FLAG_ENTRIES_NEED_REFRESH;
8259+
}
8260+
82058261
bool menu_driver_iterate(
82068262
struct menu_state *menu_st,
82078263
gfx_display_t *p_disp,
@@ -8210,6 +8266,8 @@ bool menu_driver_iterate(
82108266
enum menu_action action,
82118267
retro_time_t current_time)
82128268
{
8269+
menu_driver_pump_pending(menu_st, settings);
8270+
82138271
return ( menu_st->driver_data
82148272
&& generic_menu_iterate(
82158273
menu_st,

playlist.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3996,6 +3996,12 @@ static bool playlist_config_matches(const playlist_config_t *a,
39963996
&& (a->autofix_paths == b->autofix_paths);
39973997
}
39983998

3999+
/* True while a deferred cached init is part way through a read. */
4000+
bool playlist_init_cached_pending(void)
4001+
{
4002+
return playlist_cached_pending != NULL;
4003+
}
4004+
39994005
void playlist_init_cached_defer_abort(void)
40004006
{
40014007
if (playlist_cached_pending)

playlist.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ void playlist_parse_abort(playlist_parse_t *p);
220220
* playlist_init_cached_defer_abort() abandons any pending parse. */
221221
int playlist_init_cached_deferred(const playlist_config_t *config,
222222
bool (*budget_cb)(void *), void *budget_ud);
223+
bool playlist_init_cached_pending(void);
223224
int playlist_init_cached_continue(bool (*budget_cb)(void *), void *budget_ud);
224225
int playlist_init_cached_finish(void);
225226
void playlist_init_cached_defer_abort(void);

samples/playlist/playlist_parity_test.c

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,6 +1192,96 @@ static void lane_saf_slow_reads(void)
11921192
yields, saf_read_calls);
11931193
}
11941194

1195+
1196+
/* The pump: a yielded parse must finish without user input.
1197+
*
1198+
* ENTRIES_NEED_REFRESH is consumed only in
1199+
* generic_menu_entry_action(), so before menu_driver_iterate() got a
1200+
* pump, a read that yielded stayed unfinished until the next
1201+
* keypress - the playlist came up empty and populated only after
1202+
* backing out and re-entering. This drives the same
1203+
* continue/finish pair the pump calls, over the slow VFS, and
1204+
* asserts that repeated frames alone complete it. */
1205+
static void lane_pump_completes_without_input(void)
1206+
{
1207+
char path_a[512];
1208+
char path_b[512];
1209+
char *doc = NULL;
1210+
playlist_config_t cfg_a;
1211+
playlist_config_t cfg_b;
1212+
playlist_t *cached = NULL;
1213+
unsigned had = failures;
1214+
unsigned frames = 0;
1215+
int r;
1216+
1217+
if (!(doc = big_fixture_doc(600, "/games/pump")))
1218+
{
1219+
CHECK(false, "pump lane: fixture alloc");
1220+
return;
1221+
}
1222+
snprintf(path_a, sizeof(path_a), "%s/pump_a.lpl", fixture_dir);
1223+
snprintf(path_b, sizeof(path_b), "%s/pump_b.lpl", fixture_dir);
1224+
CHECK(write_whole(path_a, doc), "fixture write");
1225+
CHECK(write_whole(path_b, doc), "fixture write");
1226+
free(doc);
1227+
1228+
config_defaults(&cfg_a, path_a);
1229+
cfg_a.capacity = 8192;
1230+
playlist_config_set_base_content_directory(&cfg_a, "/games/pump");
1231+
config_defaults(&cfg_b, path_b);
1232+
cfg_b.capacity = 8192;
1233+
playlist_config_set_base_content_directory(&cfg_b, "/games/pump");
1234+
1235+
playlist_free_cached();
1236+
saf_vfs_install();
1237+
1238+
CHECK(playlist_init_cached(&cfg_a), "pump lane: first init");
1239+
1240+
/* The first touch, as the displaylist makes it: one budgeted
1241+
* slice, which over SAF-speed reads does not finish. */
1242+
{
1243+
int k = 2;
1244+
r = playlist_init_cached_deferred(&cfg_b, budget_countdown, &k);
1245+
}
1246+
CHECK(r == 0,
1247+
"pump lane: the first slice finished the read, so this lane "
1248+
"is not exercising the case it exists for");
1249+
CHECK(playlist_init_cached_pending(),
1250+
"pump lane: nothing reported as pending after a yield");
1251+
1252+
/* Now only frames - no further requests, no input at all. */
1253+
while (playlist_init_cached_pending() && frames < 10000)
1254+
{
1255+
int k = 2;
1256+
frames++;
1257+
r = playlist_init_cached_continue(budget_countdown, &k);
1258+
if (r > 0)
1259+
playlist_init_cached_finish();
1260+
else if (r < 0)
1261+
break;
1262+
}
1263+
1264+
CHECK(frames > 0, "pump lane: no frames were needed");
1265+
CHECK(!playlist_init_cached_pending(),
1266+
"pump lane: still pending after %u frames - a read that "
1267+
"yields never completes on its own", frames);
1268+
1269+
cached = playlist_get_cached();
1270+
CHECK(cached && streq(playlist_get_conf_path(cached), path_b),
1271+
"pump lane: the pump did not install the requested "
1272+
"playlist");
1273+
CHECK(cached && playlist_size(cached) == 600,
1274+
"pump lane: %u entries, wanted 600",
1275+
cached ? (unsigned)playlist_size(cached) : 0);
1276+
1277+
saf_vfs_remove();
1278+
playlist_free_cached();
1279+
1280+
if (failures == had)
1281+
fprintf(stderr, "[pass] pump lane (%u frames, no input)\n",
1282+
frames);
1283+
}
1284+
11951285
int main(int argc, char *argv[])
11961286
{
11971287
char cmd[600];
@@ -1224,6 +1314,7 @@ int main(int argc, char *argv[])
12241314
lane_switch_playlist_never_shows_previous();
12251315
lane_switch_supersedes_pending();
12261316
lane_saf_slow_reads();
1317+
lane_pump_completes_without_input();
12271318

12281319
snprintf(cmd, sizeof(cmd), "rm -rf %s", fixture_dir);
12291320
if (system(cmd) != 0) { }

0 commit comments

Comments
 (0)