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

VIDEO: open latency, spurious unrecognized events #370

Closed
wants to merge 1 commit into from

Conversation

bscottm
Copy link
Contributor

@bscottm bscottm commented Mar 20, 2024

Open latency: Encountered in Windows Subsystem for Linux (WSL). WSL graphics initialization takes more than 20x100ms (2 seconds). It's a non-deterministic delay accrued across OS thread scheduling, WSL's window emulation system and the underlying video event processing loop's execution.

Accomodate the non-determinism via a condition variable and associated mutex to signal EVENT_OPEN's completion.

  • The EVENT_OPEN-generating thread initializes a startup structure that contains the condition variable and mutex, which is passed to the EVENT_OPEN event. The thread then waits for the condition to signal via SDL_CondWait().

  • The EVENT_OPEN event processing queues EVENT_OPENCOMPLETE before returning.

  • EVENT_OPENCOMPLETE signals the condition variable and the SDL_CondWait()-ing thread resumes.

Unrecognized events: PDP-11's VT demo mode ("lunar lander") generates messages when emitting the bell (beep). These messages are attributed to a SDL issue (presumably prior to SDL2 2.0.6.)

EVENT_REDRAW optimization generates these spurious messages via the vid_get_event_window() function, which reports an unrecognized SDL event if it cannot associate a VID_DISPLAY with an event's window identifier. Not all SIMH video events require a non-NULL VID_DISPLAY pointer.

  • Separate unrecognized event reporting from vid_get_event_window(). If a function expects a valid VID_DISPLAY ptr and gets NULL, then it should report the error via reportUnrecognizedEvent().

  • vid_video_events(): Simplified -- process one event at a time. A function dispatch table drives SIMH video event processing.

  • Separate redraws from SIMH events so that SDL_PeepEvents() filters ONLY on redraw events. Simplifies coalescing duplicate redraw events for the same window.

  • Redraw frame rate: Coalesce redraw requests if they arrive faster than human perception (30 fps) into a deferred redraw event queued by a SDL_AddTimer() callback. Skip window refresh requests if a deferred redraw request is pending.

Functional changes:

  • vid_active: Changed to SDL_atomic_t (from int) to ensure correct cross-thread writes. Actually encountered on AARCH64.

  • vid_is_active(): vid_active accessor function. Updated references in VAX/vax4xx_{va,vc,ve}.c, VAX/vax_{va,vc}.c.

  • VID_DISPLAY::vid_ready: Changed to SDL_atomic_t (from t_bool) to ensure correct cross-thread writes. Actually encountered on AARCH64.

  • sim_video_system_init(): Consolidated SDL2 initialiation function. Allocates the simh_user_event and simh_redraw_event identifiers, initializes the event and window event debug strings, calls SDL_Init().

  • Use the newer audio API when SDL version >= 2.0.6, as the SDL documentation strongly suggests and future-proof the code.

  • vid_beep_cond, vid_beep_mutex: SIMH is supposed to wait until the entirety of a audio bell plays out. Instead of using SDL_Delay(), use condition variable and mutex to wait exactly the duration needed.

Cosmetic changes:

  • Window event, event and user event debugging functions: Initialization code moved out of vid_video_events(). reportUnrecognizedEvent() reports developer-friendly output.

  • simh_user_event, simh_redraw_event: SDL_RegisterEvents()-generated event identifiers for SIMH events, vice using the SDL_USEREVENT constant. SDL_RegisterEvents() is the preferred method for defining application-specific events (RTFM SDL_PushEvent().)

Open latency: Encountered in Windows Subsystem for Linux (WSL). WSL
graphics initialization takes more than 20x100ms (2 seconds).  It's a
non-deterministic delay accrued across OS thread scheduling, WSL's
window emulation system and the underlying video event processing loop's
execution.

Accomodate the non-determinism via a condition variable and associated
mutex to signal EVENT_OPEN's completion.

  - The EVENT_OPEN-generating thread initializes a startup structure
    that contains the condition variable and mutex, which is passed to
    the EVENT_OPEN event. The thread then waits for the condition to
    signal via SDL_CondWait().

  - The EVENT_OPEN event processing queues EVENT_OPENCOMPLETE before
    returning.

  - EVENT_OPENCOMPLETE signals the condition variable and the
    SDL_CondWait()-ing thread resumes.

Unrecognized events: PDP-11's VT demo mode ("lunar lander") generates
messages when emitting the bell (beep).  These messages are attributed
to a SDL issue (presumably prior to SDL2 2.0.6.)

EVENT_REDRAW optimization generates these spurious messages via the
vid_get_event_window() function, which reports an unrecognized SDL event
if it cannot associate a VID_DISPLAY with an event's window identifier.
Not all SIMH video events require a non-NULL VID_DISPLAY pointer.

  - Separate unrecognized event reporting from vid_get_event_window().
    If a function expects a valid VID_DISPLAY ptr and gets NULL, then
    it should report the error via reportUnrecognizedEvent().

  - vid_video_events(): Simplified -- process one event at a time. A
    function dispatch table drives SIMH video event processing.

  - Separate redraws from SIMH events so that SDL_PeepEvents() filters
    ONLY on redraw events. Simplifies coalescing duplicate redraw events
    for the same window.

  - Redraw frame rate: Coalesce redraw requests if they arrive faster
    than human perception (30 fps) into a deferred redraw event queued
    by a SDL_AddTimer() callback. Skip window refresh requests if a
    deferred redraw request is pending.

Functional changes:

- vid_active: Changed to SDL_atomic_t (from int) to ensure correct
  cross-thread writes. Actually encountered on AARCH64.

- vid_is_active(): vid_active accessor function.  Updated references in
  VAX/vax4xx_{va,vc,ve}.c, VAX/vax_{va,vc}.c.

- VID_DISPLAY::vid_ready: Changed to SDL_atomic_t (from t_bool) to
  ensure correct cross-thread writes. Actually encountered on AARCH64.

- sim_video_system_init(): Consolidated SDL2 initialiation function.
  Allocates the simh_user_event and simh_redraw_event identifiers,
  initializes the event and window event debug strings, calls
  SDL_Init().

- Use the newer audio API when SDL version >= 2.0.6, as the SDL
  documentation strongly suggests and future-proof the code.

- vid_beep_cond, vid_beep_mutex: SIMH is supposed to wait until the
  entirety of a audio bell plays out. Instead of using SDL_Delay(), use
  condition variable and mutex to wait exactly the duration needed.

Cosmetic changes:

- Window event, event and user event debugging functions: Initialization
  code moved out of vid_video_events(). reportUnrecognizedEvent()
  reports developer-friendly output.

- simh_user_event, simh_redraw_event: SDL_RegisterEvents()-generated
  event identifiers for SIMH events, vice using the SDL_USEREVENT
  constant. SDL_RegisterEvents() is the preferred method for defining
  application-specific events (RTFM SDL_PushEvent().)
@bscottm
Copy link
Contributor Author

bscottm commented Mar 20, 2024

@pkoning2: Apologies for the larger-than-normal PR. One thing led to another -- WSL window latency causing seg faults and getting annoyed by the spurious unrecognized event messages in the PDP-11 VT demo. It appears larger due to code shifting, primarily the message array initialization (readability) and table-driven SIMH (SDL user) event dispatch (simplification.)

I have tested on Windows and RPi 64-bit Linux, which exorcises two implementations of the SDL event loop, with PDP-11, microvax2 QVSS and microvax2000 QVSS (not graphics heavy, but the text renders correctly and the cursor is in the correct place.) Have not tested the BESM-6 simulator or on macOS.

I can attach/post video debugging (set vt enable debug=vvid) output to show the coalesced redraws, if that would help.

@bscottm
Copy link
Contributor Author

bscottm commented Mar 21, 2024

@pkoning2: One thing that still bugs me a bit -- the 400ms (give or take) wait for the "beep" to complete. I've preserved that behavior, but it just doesn't seem correct behavior. The "beep" playout should be smarter to keep track of pending beeps, introduce a small pause between pending beeps and let the rest of the video support do its thing (like painting the lunar explosion.)

@bscottm
Copy link
Contributor Author

bscottm commented Mar 21, 2024

Pulling this back to address thread data races identified by the Clang sanitizer. SDL is highly threaded underneath its implementation and there is a lot of cross-thread data sharing between SIMH and the SDL event loop.

@bscottm bscottm closed this Mar 21, 2024
@bscottm bscottm deleted the scp-video-wsl branch March 30, 2024 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant