From faad8ce26a4dffa788887ebf1a78f31040b30642 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Mon, 6 Jul 2026 13:22:31 -0400 Subject: [PATCH] screenshare: honor viewer settings for the VLC player too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The viewer playback settings (hardware decode + buffering) only shaped mpv's argv; vlc_args() was fixed, so a VLC viewer silently ignored them. The load-bearing case is hardware decode: mpv defaults to software decode (the A-bug fix), but VLC hardware-decodes by default, so a VLC viewer with the default hardware_decode=false still got GPU decode and could hit the frame-1 freeze the default exists to avoid — the toggle did nothing. vlc_args() now takes the settings and maps the knobs that translate cleanly to VLC: hardware decode (--avcodec-hw=none/any) and buffering posture (network/live caching ms). The genuinely mpv-specific knobs (cache_mb byte-cache, extra_mpv_args) stay mpv-only; the Settings UI hints are reworded to say which knobs are mpv-only vs universal. +2 tests. Co-Authored-By: Claude Opus 4.8 --- src/app/mod.rs | 6 ++-- src/screenshare/mod.rs | 69 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 49f7176..f95bc5e 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -3452,7 +3452,7 @@ fn share_player_hint(player: SharePlayer) -> &'static str { fn share_buffering_hint(buffering: ShareBuffering) -> &'static str { match buffering { ShareBuffering::LowLatency => "Small buffers for interactive screen sharing.", - ShareBuffering::Smooth => "Larger mpv cache/readahead for steadier playback.", + ShareBuffering::Smooth => "Larger cache/readahead for steadier playback.", } } @@ -5146,13 +5146,13 @@ fn view(state: &AppState) -> Element<'_, AppMessage> { Some(screen_share.cache_mb), AppMessage::ScreenShareCacheMbSelected, ).width(iced::Length::Fill), - text("Used as mpv demuxer cache size.").size(11).color(color_subtext), + text("mpv demuxer cache size (mpv only).").size(11).color(color_subtext), ].spacing(4).width(iced::Length::Fill), column![ checkbox(screen_share.hardware_decode) .label("Hardware video decode") .on_toggle(AppMessage::ToggleScreenShareHardwareDecode), - text("Adds --hwdec=auto to mpv. Off avoids the known frame-freeze bug.").size(11).color(color_subtext), + text("GPU decode (mpv --hwdec=auto / VLC hardware decode). Off avoids the known frame-freeze bug.").size(11).color(color_subtext), ].spacing(8).width(iced::Length::Fill), ].spacing(16).width(iced::Length::Fill), ].spacing(10).width(iced::Length::Fill), diff --git a/src/screenshare/mod.rs b/src/screenshare/mod.rs index fbf9955..79da653 100644 --- a/src/screenshare/mod.rs +++ b/src/screenshare/mod.rs @@ -608,7 +608,7 @@ fn event_for_log(ev: &PixelpassEvent) -> String { /// kept playing. fn launch_player(url: &str, settings: &ScreenShareSettings) -> std::io::Result<()> { let mpv_args = mpv_args(settings); - let vlc_args = vlc_args(); + let vlc_args = vlc_args(settings); let first = match settings.player { SharePlayer::Mpv => ("mpv", &mpv_args), SharePlayer::Vlc => ("vlc", &vlc_args), @@ -655,10 +655,33 @@ pub fn mpv_args(settings: &ScreenShareSettings) -> Vec { args } -fn vlc_args() -> Vec { +/// Build the argv for a VLC viewer. VLC honors the subset of viewer settings +/// that map cleanly onto its option set: the buffering posture (network/live +/// caching, in ms) and hardware decoding. The rest of the viewer knobs are +/// mpv-specific — `cache_mb` is an mpv demuxer *byte* cache (VLC's caching is +/// time-based, already covered by `buffering`) and `extra_mpv_args` is literally +/// mpv flags — so they are deliberately not mapped here; the Settings UI labels +/// them as mpv-only. Pure: no I/O. +/// +/// The hardware-decode mapping is the load-bearing one: VLC hardware-decodes by +/// default, so without an explicit `--avcodec-hw=none` a VLC viewer would ignore +/// the (default-off) hardware-decode toggle and could hit the frame-1 freeze +/// that default exists to avoid — the same A-bug that made us drop mpv's forced +/// `--hwdec=auto`. +fn vlc_args(settings: &ScreenShareSettings) -> Vec { + let caching_ms = match settings.buffering { + ShareBuffering::LowLatency => 200, + ShareBuffering::Smooth => 1500, + }; + let hw = if settings.hardware_decode { + "--avcodec-hw=any" + } else { + "--avcodec-hw=none" + }; vec![ - "--network-caching=200".to_string(), - "--live-caching=200".to_string(), + format!("--network-caching={caching_ms}"), + format!("--live-caching={caching_ms}"), + hw.to_string(), ] } @@ -820,6 +843,44 @@ mod tests { ); } + #[test] + fn vlc_args_default_disables_hardware_decode() { + // The A-bug fix default (hardware_decode = false) must reach VLC too: + // VLC hardware-decodes by default, so without an explicit + // `--avcodec-hw=none` a VLC viewer would ignore the toggle and could hit + // the frame-1 freeze. Low-latency buffering keeps the 200 ms caches. + assert_eq!( + vlc_args(&ScreenShareSettings::default()), + vec![ + "--network-caching=200", + "--live-caching=200", + "--avcodec-hw=none", + ] + ); + } + + #[test] + fn vlc_args_smooth_buffering_and_hwdecode() { + // Enabling hardware decode flips VLC to `--avcodec-hw=any`; Smooth + // buffering raises the network/live caches. cache_mb / extra_mpv_args are + // mpv-only and must NOT leak into the VLC argv. + let settings = ScreenShareSettings { + hardware_decode: true, + buffering: ShareBuffering::Smooth, + cache_mb: 16, + extra_mpv_args: "--no-osc".to_string(), + ..ScreenShareSettings::default() + }; + assert_eq!( + vlc_args(&settings), + vec![ + "--network-caching=1500", + "--live-caching=1500", + "--avcodec-hw=any", + ] + ); + } + #[test] fn sanitize_app_name_trims_and_rejects_garbage() { assert_eq!(