Calls lavf_options() post mpv.MPV() init and writes each entry into
the demuxer-lavf-o property. This is the consumer side of the split
helpers introduced in the previous commit. Verified end-to-end by
launching the GUI: mpv constructs cleanly and m['demuxer-lavf-o']
reads back as {'protocol_whitelist': 'file,http,https,tls,tcp'}.
Audit-Ref: SECURITY_AUDIT.md finding #2
Severity: High
The previous attempt set ``demuxer_lavf_o`` as an init kwarg with a
comma-laden ``protocol_whitelist=file,http,https,tls,tcp`` value.
mpv rejected it with -7 OPT_FORMAT because python-mpv's init path
goes through ``mpv_set_option_string``, which routes through mpv's
keyvalue list parser — that parser splits on ``,`` to find entries,
shredding the protocol list into orphan tokens. Backslash-escaping
``\,`` did not unescape on this code path either.
Splits the option set into two helpers:
- ``build_mpv_kwargs`` — init kwargs only (ytdl=no, load_scripts=no,
POSIX input_conf null, all the existing playback/audio/network
tuning). The lavf option is intentionally absent.
- ``lavf_options`` — a dict applied post-construction via the
python-mpv property API, which uses the node API and accepts
dict values for keyvalue-list options without splitting on
commas inside the value.
Tests cover both paths: that ``demuxer_lavf_o`` is NOT in the init
kwargs (regression guard), and that ``lavf_options`` returns the
expected protocol set.
Audit-Ref: SECURITY_AUDIT.md finding #2
Severity: High
Replaces the inline mpv.MPV(...) literal kwargs with a call through
build_mpv_kwargs(), which adds ytdl=no, load_scripts=no, a lavf
protocol whitelist (file,http,https,tls,tcp), and POSIX input_conf
lockdown. Closes the yt-dlp delegation surface (CVE-prone extractors
invoked on attacker-supplied URLs) and the concat:/subfile: local-
file-read gadget via ffmpeg's lavf demuxer.
behavior change from v0.2.5: any file_url whose host is only
handled by yt-dlp (youtube.com, reddit.com, etc.) will no longer
play. Boorus do not legitimately return such URLs, so in practice
this only affects hostile responses. Cached local files and direct
https .mp4/.webm/.mkv continue to work.
Manually smoke tested: played a cached local .mp4 from the library
(file: protocol) and a fresh network .webm from a danbooru search
(https: protocol) — both work.
Audit-Ref: SECURITY_AUDIT.md finding #2
Severity: High
Extracts the mpv.MPV() kwargs into a Qt-free pure function so the
security-relevant options can be unit-tested on CI (which lacks
PySide6 and libmpv). The builder embeds the audit #2 hardening —
ytdl="no", load_scripts="no", and a lavf protocol whitelist of
file,http,https,tls,tcp — alongside the existing playback tuning.
Not yet wired into _MpvGLWidget; that lands in the next commit.
Audit-Ref: SECURITY_AUDIT.md finding #2
Severity: High
- Render "Once" loop icon as bold "1×" text via QPainter drawText
instead of the hand-drawn line art
- Responsive controls bar: hide volume slider below 320px, duration
label below 240px, current time label below 200px
- _toggle_play seeks to 0 if paused at EOF so pressing play replays
the video in Once mode instead of doing nothing
- Fix stray "Auto" text leaking through the autoplay icon — the
autoplay property setter was still calling setText
Logs when GL render context is actually initialized (not on the no-op
path). Confirms GL init fires once per widget lifetime, not on every
video click. Kept permanently for future debugging.
Replaces the parallel httpx download with mpv's stream-record per-file
option. When play_file receives an HTTP URL, it passes stream_record
pointing at a .part temp file alongside the URL. mpv writes the incoming
network stream to disk as it decodes, so a single HTTP connection serves
both playback and cache population.
On clean EOF the .part is promoted to the real cache path via os.replace.
Seeks invalidate the recording (mpv may skip byte ranges), so
_seeked_during_record flags it for discard. stop() and rapid-click
cleanup also discard incomplete .part files.
At this commit both pipelines are active — _load still runs the httpx
download in parallel. Whichever finishes second wins os.replace. The
next commit removes the httpx path.
behavior change: mpv now uses explicit cache=yes, cache_pause=no
(stutter over pause for short clips), 50MiB demuxer buffer cap,
20s read-ahead, and 10s network timeout (down from ~60s default).
Improves first-frame latency on uncached video streams and surfaces
stalled-connection errors faster.
The 500ms `_seek_pending_until` pin window in `VideoPlayer._poll`
became redundant after `609066c` switched the slider seek from
`'absolute'` to `'absolute+exact'`. With exact seek, mpv decodes
from the previous keyframe forward to the click position before
reporting it via `time_pos`, so `_poll`'s read-and-write loop
naturally lands the slider at the click position without any
pinning. The pin was defense in depth for keyframe-rounding latency
that no longer exists.
Removed:
- `_seek_target_ms`, `_seek_pending_until`, `_seek_pin_window_secs`
fields from `__init__`
- The `_time.monotonic() < _seek_pending_until` branch in `_poll`
(now unconditionally `setValue(pos_ms)` after the isSliderDown
check)
- The pin-arming logic from `_seek` (now just calls `mpv.seek`
directly)
Net diff: ~30 lines removed, ~10 lines of explanatory comments
added pointing future-pax at the `609066c` commit body for the
"why" of the cleanup.
The popout's state machine SeekingVideo state continues to track
seeks via the dispatch path (seek_target_ms is held on the state
machine, not on VideoPlayer) for the future when the adapter's
SeekVideoTo apply handler grows past its current no-op. The
removal here doesn't affect that — it only drops dead defense-in-
depth code from the legacy slider rendering path.
Verification:
- Phase A test suite (16 tests) still passes
- State machine tests (65 tests) still pass — none of them touch
VideoPlayer fields
- Both surfaces (embedded preview + popout) still seek correctly
per the post-609066c verification (commit 14a/14b sweep)
Followup target from docs/POPOUT_FINAL.md "What's NOT done"
section. The other listed followup (replace self._viewport with
self._state_machine.viewport in popout/window.py) is bigger and
filed for a future session.
The slider's _seek used plain 'absolute' (keyframe seek), which made
mpv land on the nearest keyframe at-or-before the click position.
For sparse-keyframe videos (1-5s GOP) the actual position landed
1-5s behind where the user clicked. The 500ms _seek_pending_until
pin window from c4061b0 papered over this for half a second, but
afterwards the slider visibly dragged back to mpv's keyframe-rounded
position and crawled forward. Observed in BOTH the embedded preview
and the popout slider — the bug lives in the shared VideoPlayer
class, so fixing it once fixes both surfaces.
Empirically verified by the pre-commit-1 mpv probe in
docs/POPOUT_REFACTOR_PLAN.md: seek(7.0, 'absolute') landed at
time_pos=5.000 (2s back); seek(12.0, 'absolute') landed at 10.033
(also 2s back). Both match the user's reported drag-back symptom.
The other seek paths in VideoPlayer already use exact mode:
- seek_to_ms (line 318): 'absolute+exact'
- _seek_relative (line 430): 'relative+exact'
The slider's _seek was the only outlier. The original c4061b0 commit
chose plain 'absolute' for "responsiveness" and added the pin window
to hide the keyframe rounding. This commit removes the underlying
cause: the seek now decodes from the previous keyframe forward to
the EXACT target position before mpv emits playback-restart, costing
~30-100ms more per seek depending on GOP density (well under the
500ms pin window) but landing time_pos at the click position
exactly. The slider doesn't need any pin window to mask a
discrepancy that no longer exists.
The _seek_pending_until pin remains in place as defense in depth —
it's now redundant for keyframe rounding but still smooths over the
sub-100ms decode latency between the click and the first _poll
tick that reads the new time_pos. Commit 14b will remove the legacy
pin code as part of the imperative-path cleanup.
This also unblocks the popout state machine's design (commits 6, 11,
14a). The state machine's SeekingVideo state lasts until mpv's
playback-restart event arrives — empirically 14-34ms in the user's
verification log of commit 14a. Without exact seek, commit 14b
would visibly REGRESS slider behavior because compute_slider_display_ms
returns mpv.time_pos after 30ms instead of the legacy 500ms — the
drag-back would surface immediately on every seek. With exact seek,
mpv.time_pos == seek_target_ms after the seek completes, so the
state machine's slider pin is correct without needing any extra
window.
Found during commit 14a verification gate. Pre-existing bug — the
state machine refactor revealed it but didn't introduce it.
Tests passing after this commit: 81 / 81 (16 Phase A + 65 state).
Phase A still green. Phase B regression target met (the dispatch
trace from the verification run shows correct SeekRequested →
SeekCompleted round-trips with no spurious state transitions).
Verification:
- Click slider mid-playback in embedded preview → no drag-back
- Click slider mid-playback in popout → no drag-back
- Drag the slider continuously → still works (isSliderDown path
unchanged)
- Period/Comma keys (relative seek) → still work (already use
'relative+exact')
Adds a Qt Signal that mirrors mpv's `playback-restart` event. The
upcoming popout state machine refactor (Prompt 3) needs a clean,
event-driven "seek/load completed" edge to drive its SeekingVideo →
PlayingVideo and LoadingVideo → PlayingVideo transitions, replacing
the current 500ms timestamp suppression window in `_seek_pending_until`.
mpv's `playback-restart` fires once after each loadfile (when playback
actually starts producing frames) and once after each completed seek.
Empirically verified by the pre-commit-1 probe in
docs/POPOUT_REFACTOR_PLAN.md: a load + 2 seeks produces exactly 3
events, with `seeking=False` at every event (the event represents the
completion edge, not the in-progress state).
The state machine adapter will distinguish "load done" from "seek done"
by checking the state machine's current state at dispatch time:
- `playback-restart` while in LoadingVideo → VideoStarted event
- `playback-restart` while in SeekingVideo → SeekCompleted event
Implementation:
- One Signal added near the existing play_next / media_ready /
video_size definitions, with a doc comment explaining what fires
it and which state machine consumes it.
- One event_callback registration in `_ensure_mpv` (alongside the
existing observe_property calls). The callback runs on mpv's
event thread; emitting a Qt Signal is thread-safe and the
receiving slot runs on the GUI thread via Qt's default
AutoConnection (sender and receiver in the same thread by the
time the popout adapter wires the connection).
- The decorator-based `@self._mpv.event_callback(...)` form is used
to match the rest of the python-mpv idioms in the file. The inner
function name `_emit_playback_restart` is local-scoped — mpv keeps
its own reference, so there's no leak from re-creation across
popout open/close cycles (each popout gets a fresh VideoPlayer
with its own _ensure_mpv call).
This is the only commit in the popout state machine refactor that
touches `media/video_player.py`. All subsequent commits land in
`popout/` (state.py, effects.py, hyprland.py, window.py) or
`gui/main_window.py` interface updates. 21 lines added, 0 removed.
Verification:
- Phase A test suite (16 tests) still passes
- Module imports cleanly with the new Signal in place
- App launches without errors (smoke test)
Test cases for state machine adapter (Prompt 3 popout/state.py):
- VideoPlayer.playback_restart fires once on play_file completion
- VideoPlayer.playback_restart fires once per _seek call
The popout's VideoPlayer is constructed with no mpv attached — mpv
gets wired up in _ensure_mpv on the first set_media call. main_window's
_open_fullscreen_preview syncs preview→popout state right after the
popout is constructed, so it writes is_muted *before* mpv exists. The
old setter only forwarded to mpv if mpv was set:
@is_muted.setter
def is_muted(self, val: bool) -> None:
if self._mpv:
self._mpv.mute = val
self._mute_btn.setText("Unmute" if val else "Mute")
For the popout's pre-mpv VideoPlayer this updated the button text but
silently dropped the value. _ensure_mpv then created the mpv instance
later with default mute=False, so the popout always opened unmuted
even when the embedded preview was muted (or when a previous popout
session had muted and then closed).
Fix: introduce a Python-side _pending_mute field that survives the
lazy mpv creation. The setter writes to _pending_mute unconditionally
and forwards to mpv if it exists. The getter returns _mpv.mute when
mpv is set, otherwise _pending_mute. _ensure_mpv replays _pending_mute
into the freshly-created mpv instance after applying the volume from
the slider, mirroring the existing volume-from-slider replay pattern
that already worked because the slider widget exists from construction
and acts as the volume's persistent storage.
Also threaded _pending_mute through _toggle_mute so the click-driven
toggle path stays consistent with the setter path — without it, a
mute toggle inside the popout would update mpv but not _pending_mute,
and the next sync round-trip via the setter would clobber it.
Verified manually:
- popout video, click mute, close popout, reopen on same video →
mute persisted (button shows "Unmute", audio silent)
- toggle to unmute, close, reopen → unmuted persisted
- embedded preview video mute → close popout → state propagates
correctly via _on_fullscreen_closed's reverse sync
Two related preservation bugs around the popout's F11 fullscreen
toggle, both surfaced during the post-refactor verification sweep.
1. ImageViewer zoom/pan loss on resize
ImageViewer.resizeEvent unconditionally called _fit_to_view() on every
resize event. F11 enter resizes the widget to the full screen, F11
exit resizes it back to the windowed size — both fired _fit_to_view,
clobbering any explicit user zoom and offset. Same problem for manual
window drags and splitter moves.
Fix: in resizeEvent, compute the previous-size fit-to-view zoom from
event.oldSize() and compare to current _zoom. Only re-fit if the user
was at fit-to-view at the previous size (within a 0.001 epsilon —
tighter than any wheel/key zoom step). Otherwise leave _zoom and
_offset alone.
The first-resize case (no valid oldSize, e.g. initial layout) still
defaults to fit, matching the original behavior for fresh widgets.
2. Popout window position lost on F11 round-trip
FullscreenPreview._enter_fullscreen captured _windowed_geometry but
the F11-exit restore goes through `_viewport` (the persistent center +
long_side that drives _fit_to_content). The drift detection in
_derive_viewport_for_fit only updates _viewport when
_last_dispatched_rect is set AND a fit is being computed — neither
path catches the "user dragged the popout with Super+drag and then
immediately pressed F11" sequence:
- Hyprland Super+drag does NOT fire Qt's moveEvent (xdg-toplevel
doesn't expose absolute screen position to clients on Wayland),
so Qt-side drift detection is dead on Hyprland.
- The Hyprland-side drift detection in _derive_viewport_for_fit
only fires inside a fit, and no fit is triggered between a drag
and F11.
- Result: _viewport still holds whatever it had before the drag —
typically the saved-from-last-session geometry seeded by the
first-fit one-shot at popout open.
When F11 exits, the deferred _fit_to_content reads the stale viewport
and restores the popout to the *previously seeded* position instead of
where the user actually had it.
Fix: in _enter_fullscreen, after capturing _windowed_geometry, also
write the current windowed state into self._viewport directly. The
viewport then holds the actual pre-fullscreen position regardless of
how it got there (drag, drag+nav, drag+F11, etc.), and F11 exit's
restore reads it correctly.
Bundled into one commit because both fixes are "F11 round-trip should
preserve where the user was" — the image fix preserves content state
(zoom/pan), the popout fix preserves window state (position). Same
theme, related root cause class. Bisecting one without the other
would be misleading.
Verified manually:
- image: scroll-zoom + drag pan + F11 + F11 → zoom and pan preserved
- image: untouched zoom + F11 + F11 → still fits to view
- image: scroll-zoom + manual window resize → zoom preserved
- popout: Super+drag to a new position + F11 + F11 → lands at the
dragged position, not at the saved-from-last-session position
- popout: same sequence on a video post → same result (videos don't
have zoom/pan, but the window-position fix applies to all media)
The seek slider snapped visually backward after a click for the first
~tens to hundreds of ms — long enough to be obvious. Race trace:
user clicks slider at target T
→ _ClickSeekSlider.mousePressEvent fires
→ setValue(T) lands the visual at the click position
→ clicked_position emits → _seek dispatches mpv.seek(T) async
→ mpv processes the seek on its own thread
meanwhile the 100ms _poll timer keeps firing
→ reads mpv.time_pos (still the OLD position, mpv hasn't caught up)
→ calls self._seek_slider.setValue(pos_ms)
→ slider visually snaps backward to the pre-seek position
→ mpv finishes seeking, next poll tick writes the new position
→ slider jumps forward to settle near T
The isSliderDown() guard at the existing setValue site (around line
425) only suppresses writebacks during a *drag* — fast clicks never
trigger isSliderDown, so the guard didn't help here.
Fix: pin the slider to the user's target throughout a 500ms post-seek
window. Mirror the existing _eof_ignore_until pattern (stale-eof
suppression in play_file → _on_eof_reached) — it's the same shape:
"after this dispatch, ignore poll-driven writebacks for N ms because
mpv hasn't caught up yet."
- _seek now records _seek_target_ms and arms _seek_pending_until
- _poll forces _seek_slider.setValue(_seek_target_ms) on every tick
inside the window, instead of mpv's lagging time_pos
- After the window expires, normal mpv-driven writes resume
Pin window is 500ms (vs the eof window's 250ms) because network and
streaming seeks take noticeably longer than local-cache seeks. Inside
the window the slider is forced to the target every tick, so mpv lag
is invisible no matter how long it takes within the window.
First attempt used a smaller 250ms window with a "close enough"
early-release condition (release suppression once mpv reports a
position within 250ms of the target). That still showed minor
track-back because the "close enough" threshold permitted writing
back a position slightly less than the target, producing a small
visible jump. The pin-to-target approach is robust against any
mpv interim position.
The time_label keeps updating to mpv's actual position throughout —
only the slider value is pinned, so the user can still see the
seek progressing in the time text.
Verified manually: clicks at start / middle / end of a video slider
all hold position cleanly. Drag still works (the isSliderDown path
is untouched). Normal playback advances smoothly (the pin window
only affects the post-seek window, not steady-state playback).
Step 5 of the gui/app.py + gui/preview.py structural refactor. Moves
the click-to-seek QSlider variant and the mpv-backed transport-control
widget into their own module under media/. The new module imports
_MpvGLWidget from .mpv_gl (sibling) instead of relying on the bare
name in the old preview.py namespace.
Address-only adjustment: the lazy `from ..core.cache import _referer_for`
inside `play_file` becomes `from ...core.cache import _referer_for`
because the new module sits one package level deeper. Same target
module, different relative-import depth — no behavior change.
preview.py grows another re-export shim line so ImagePreview (still
in preview.py) and FullscreenPreview can keep constructing
VideoPlayer unchanged. Shim removed in commit 14.
Step 4 of the gui/app.py + gui/preview.py structural refactor. Pure
move: the OpenGL render-context host and its concrete QOpenGLWidget
companion are now in their own module under media/. The mid-file
`from PySide6.QtOpenGLWidgets import QOpenGLWidget as _QOpenGLWidget`
import that used to sit between the two classes moves with them to
the new module's import header. preview.py grows another re-export
shim line so VideoPlayer (still in preview.py) can keep constructing
_MpvGLWidget unchanged. Shim removed in commit 14.
Step 3 of the gui/app.py + gui/preview.py structural refactor. Pure
move: the zoom/pan image viewer class is now in its own module under
media/. preview.py grows another re-export shim line so ImagePreview
and FullscreenPreview (both still in preview.py) can keep constructing
ImageViewer instances unchanged. Shim removed in commit 14.
Step 1 of the gui/app.py + gui/preview.py structural refactor. Pure
move: the constant and predicate are now in their own module, and
preview.py grows a re-export shim at the bottom so existing imports
(app.py:1351 and the in-file class methods) keep working unchanged.
Shim is removed in commit 14 once importers update to the canonical
path. See docs/REFACTOR_PLAN.md for the full migration order.
