videoprocessor: fix crashes with modern V4L2 devices, detect capture resolutions and frame rates#19163
Draft
Xitee1 wants to merge 3 commits into
Draft
videoprocessor: fix crashes with modern V4L2 devices, detect capture resolutions and frame rates#19163Xitee1 wants to merge 3 commits into
Xitee1 wants to merge 3 commits into
Conversation
The video processor core was unusable with current V4L2 capture devices such as UVC/HDMI USB grabbers (issue libretro#16457): - VIDIOC_S_FMT does not fail on an unsupported resolution, it silently clamps it. Derive all buffer dimensions from the format the driver actually granted (respecting the granted field type, whose height is already per-field for TOP/BOTTOM/ALTERNATE), and clamp the capture memcpy, fixing a heap overflow in source_v4l2_normal/alternate_hack. - VIDIOC_G_STD/VIDIOC_ENUMSTD failures are now non-fatal: modern capture devices don't implement analog TV standards. video_standard is reset on every load so a previous device's standard cannot leak into retro_get_system_av_info/retro_get_region after switching devices at runtime. - Audio capture is now optional: a "none" option is offered (and is the default) so the core can start without an audio capture device. - A video-only option change keeps the ALSA device open instead of closing and immediately reopening it, which raced the async capture callback and failed with EBUSY. Switching the video device to "dummy" still tears audio down. - Runtime option changes are more robust: a failed reload outputs blank frames instead of dereferencing freed buffers, load failures unregister the audio callback before closing the ALSA handle (use after free) and reset the capture buffer state (double munmap), and the frontend is notified via RETRO_ENVIRONMENT_SET_SYSTEM_AV_INFO after a successful reconfigure so geometry/timing changes take effect. Output mode changes now trigger a reconfigure at all (the variable was checked but never fetched). - Misc: fix NULL dereferences when core variables are unset, validate the capture resolution string, drop a duplicate colorspace assignment, C89 fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the fixed capture-resolution table with the modes the capture device actually reports: the selected device (or every /dev/video* node while none is selected yet) is probed once via VIDIOC_ENUM_FMT/VIDIOC_ENUM_FRAMESIZES (discrete sizes are listed directly, stepwise/continuous ranges offer the built-in table entries that fall inside the range), deduplicated, sorted and offered as the "Capture resolution" core option. The built-in table remains as a fallback when nothing could be probed, and 720x576 (PAL) is added to it. Add a "Capture frame rate" core option driven by VIDIOC_G/S_PARM (timeperframe): the chosen rate is applied where supported and the granted rate is read back, so the timing reported to the frontend matches the real buffer delivery rate. For modern UVC/HDMI capture devices without analog TV standards this is the only rate source; analog devices keep using the TV standard's field rate as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-up to the previous two commits: - Clamp v4l2_ncapbuf to VIDEO_BUFFERS_MAX: VIDIOC_REQBUFS may grant more buffers than requested (drivers can require 3+), which overflowed v4l2_capbuf[] and corrupted adjacent state. - Always assign geometry.aspect_ratio: when VIDIOC_CROPCAP fails (common on UVC grabbers and v4l2loopback) uninitialized stack data was pushed to the frontend through SET_SYSTEM_AV_INFO on option changes. Also guard against a zero pixelaspect from the driver. - Serialize audio_handle between the frontend's async audio callback and teardown with a mutex instead of a 5ms sleep (close could still race a blocked snd_pcm_readi). While no device is open the callback sleeps, because it cannot be unregistered and the frontend's audio thread otherwise busy-spins at 100% CPU after switching audio to "none". - Key the probed resolution-list cache on the device selection: the function-local static survives core restarts in the statically linked build, so the list never refreshed after switching devices. Also trim the leading space after the "Video capture device; " label, which made probing silently skip the first enumerated device, and never register core options with a NULL value list. - Double the reported frame rate only when the driver actually granted V4L2_FIELD_INTERLACED; requesting "interlaced" from a device that clamps to progressive paced the frontend at twice the delivery rate. - Swap the ALSA device in place on audio-only option changes instead of rebuilding the whole video pipeline, and push SET_SYSTEM_AV_INFO (a full frontend AV reinit) only when av_info actually changed. - retro_reset: route through unload/load; closing and reopening the raw fds left the V4L2 stream dead until the next option change. - Replace the device_reloading global with a load_game_internal (allow_fallback) parameter and deduplicate the cleanup paths. - Guard copy_frame_cap and alternate_hack against a failed re-mmap. - Fix leaked ALSA hint strings in enumerate_audio_devices, use a neutral OSD message for audio failures (the old one always claimed "busy"), and tidy setup_dummy_source. - Standalone build: link the compat strlcpy fallback (linking was broken for any strlcpy user) and libpthread. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
|
I made a quick test and does not crash anymore ! I could load shaders also ! ping @kokoko3k |
Author
|
Nice to hear that it works for you too! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The built-in video processor core has been broken with modern V4L2 capture devices (UVC/HDMI USB grabbers) for a long time (#16457). This PR fixes the crashes and device handling, and adds runtime detection of the device's real capture resolutions and frame rates. Three commits:
videoprocessor: fix crashes and device handling with modern V4L2 devices
VIDIOC_S_FMTsilently clamps unsupported resolutions) and clamp the capturememcpy— fixes the heap overflow that crashed the core on startup.VIDIOC_G_STD/VIDIOC_ENUMSTDfailures as non-fatal (modern devices implement no analog TV standards) and reset the standard on every load so it can't leak across device switches.noneoption (default), audio failures never take video down (on-screen message instead). The ALSA handle is only published to the async audio callback once fully configured — RetroArch ignoresSET_AUDIO_CALLBACK(NULL), and the permanently-registered callback used to race the setup, making every audio device change fail withEBUSY.EBUSY); switching the video device todummystill tears audio down.munmap, blank frames instead of use-after-free after a failed reload,RETRO_ENVIRONMENT_SET_SYSTEM_AV_INFOis pushed after reconfigures, and the output mode variable is actually fetched now (changes were silently ignored before).videoprocessor: detect capture resolutions and frame rates at runtime
VIDIOC_ENUM_FMT/VIDIOC_ENUM_FRAMESIZESand offer its real modes as the "Capture resolution" list (deduplicated and sorted; the built-in table remains as fallback, with 720x576 added).VIDIOC_G_PARM/VIDIOC_S_PARM; the granted rate is read back so the timing reported to the frontend matches the real delivery rate — the only rate source on devices without analog standards.videoprocessor: harden buffer handling, audio teardown and reconfigures (review follow-up)
VIDIOC_REQBUFScount — drivers may return more buffers than requested, which overflowed the capture buffer array.VIDIOC_CROPCAPfails, e.g. v4l2loopback), an uninitialized value reached the frontend on runtime option changes.SET_SYSTEM_AV_INFO(a full AV reinit) is only pushed when geometry/timing actually changed.retro_reset) now fully reinitializes the capture stream — it previously left the core on a dead stream until the next option change.strlcpyfallback was never linked) — this also affected the pre-PR code.Testing
All builds verified warning-free (
-Wall -Wextra, C89 and C++ compatible, standalone +RARCH_INTERNAL, with and without ALSA), plus a headless ASan run against real hardware (dummy source, fallback paths, runtime reconfigures,retro_reset) with no findings.Manual test walkthrough (RetroArch on Linux; tested with a UVC HDMI grabber and a Logitech C920):
Startup
v4l2-ctl -d /dev/videoN --list-formats-ext)dummy(intended)Runtime option changes (Quick Menu → Core Options)
dummyand back: test pattern ↔ live captureAudio
none→ real device: audio starts (video briefly reinitializes — this path reloads)nonewhile running: video keeps running untouched, audio follows, repeated toggles never fail withEBUSYnone: no CPU core pinned at 100% (checktop)Lifecycle
Known behavior (out of scope): hot-unplugging the device freezes on the last frame until it is re-selected in the options; the resolution list updates on the next core start after a device switch (not live); analog cards (bttv/saa7134) were not available for testing — field handling follows the V4L2 spec, testers welcome. Possible follow-up: enumerate frame rates via
VIDIOC_ENUM_FRAMEINTERVALS.Related Issues
Related Pull Requests
Builds on #17799 / #17803, which added resolution options but did not fix the crashes.