Skip to content

video_renderer: serialize renderer access across threads - #600

Open
danieldhaiti wants to merge 1 commit into
FDH2:masterfrom
danieldhaiti:fix-renderer-lifecycle-lock
Open

danieldhaiti wants to merge 1 commit into
FDH2:masterfrom
danieldhaiti:fix-renderer-lifecycle-lock

Conversation

@danieldhaiti

Copy link
Copy Markdown

The mirror thread (codec choice, frame push), the http thread (HLS control) and the main loop (bus callback, resets) all touch renderer, renderer_type[] and hls_video with no locking. b3202df NULLs destroyed renderers, but another thread can still check a renderer and then use it after it is freed (segfault in video_renderer_choose_codec when iOS re-SETUPs mirror right after HLS /stop).

  • One recursive lock held by every public renderer function and the bus callback (g_autoptr(GRecMutexLocker), needs GLib >= 2.60).
  • On /stop, replace the player with the normal renderer straight away (as already done at end of video).
  • g_assert(!hls_video) in choose_codec becomes a logged return -1: a mirror stream arriving before the HLS player is replaced drops the connection instead of aborting.

Tested: builds on Ubuntu (GStreamer 1.28, Release), no new warnings. 200 concurrent /stop requests caused 200 inline rebuilds with no crash or assert and a clean exit. I could not drive a real mirror session from the test rig, so the original race was verified on an appliance with an iPhone (earlier fork base), not reproduced here. Complements the restart-request PR; no conflicts when both are applied.

The mirror thread (codec choice, frame push), the http thread (HLS
control) and the main loop (bus callback, resets) all touch renderer,
renderer_type[] and hls_video with no locking, so video_renderer_choose_codec()
could use a renderer being freed on another thread.

Serialize every public renderer function and the bus callback with one
recursive lock. On /stop, replace the player with the normal renderer
immediately (as already done at end of video), since the client often
starts mirroring right after /stop. A mirror stream arriving before the
HLS player is replaced now logs and drops the connection instead of
aborting on g_assert.
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.

1 participant