Add playback speed control and timeline seek hotkeys to VideoViewer - #2000
Open
v-shift-bit wants to merge 1 commit into
Open
v-shift-bit wants to merge 1 commit into
v-shift-bit wants to merge 1 commit into
Conversation
Reviewer's GuideThe VideoViewer gains a persisted playback-speed selector and keyboard timeline navigation, implemented entirely in the WPF viewer by applying MediaUriElement.SpeedRatio, manipulating MediaPosition in 100ns ticks, and capturing the new shortcuts through the focused panel. Sequence diagram for VideoViewer speed control and timeline hotkeyssequenceDiagram
participant User
participant ViewerPanel
participant SettingHelper
participant MediaUriElement
User->>ViewerPanel: PreviewKeyDown
alt Shift+Left/Right or Ctrl+Left/Right
ViewerPanel->>MediaUriElement: Seek(deltaTicks)
ViewerPanel->>MediaUriElement: SeekTo(clampedPosition)
ViewerPanel->>MediaUriElement: set MediaPosition
else Home or End
ViewerPanel->>MediaUriElement: SeekTo(0 or MediaDuration)
ViewerPanel->>MediaUriElement: set MediaPosition
else +/- or 0
ViewerPanel->>ViewerPanel: CycleSpeed(direction)
ViewerPanel->>MediaUriElement: set SpeedRatio
end
User->>ViewerPanel: LoadAndPlay(path)
ViewerPanel->>SettingHelper: Get PlaybackSpeed
SettingHelper-->>ViewerPanel: persisted speed
ViewerPanel->>MediaUriElement: set SpeedRatio
ViewerPanel->>MediaUriElement: Play()
User->>ViewerPanel: Dispose()
ViewerPanel->>SettingHelper: Set PlaybackSpeed
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="QuickLook.Plugin/QuickLook.Plugin.VideoViewer/ViewerPanel.xaml.cs" line_range="319" />
<code_context>
+ break;
+
+ case Key.OemPlus or Key.Add when modifiers == ModifierKeys.None:
+ CycleSpeed(1);
+ e.Handled = true;
+ break;
+
+ case Key.OemMinus or Key.Subtract when modifiers == ModifierKeys.None:
</code_context>
<issue_to_address>
**issue (bug_risk):** The `+` playback-speed shortcut does not work on a standard keyboard because `+` is produced as `Shift+OemPlus`, while this branch only matches `OemPlus` when `ModifierKeys.None` is active.
**Triggers:** When the user presses the standard keyboard `+` key rather than NumPad `Add`.
**Suggested fix:** Accept `ModifierKeys.Shift` for `Key.OemPlus` (while retaining the unmodified `Key.Add` case for the numeric keypad).
```suggestion
case Key.OemPlus when modifiers == ModifierKeys.Shift:
case Key.Add when modifiers == ModifierKeys.None:
```
</issue_to_address>
### Comment 2
<location path="QuickLook.Plugin/QuickLook.Plugin.VideoViewer/ViewerPanel.xaml.cs" line_range="315-316" />
<code_context>
+ break;
+
+ case Key.End when modifiers == ModifierKeys.None:
+ SeekTo(mediaElement.MediaDuration);
+ e.Handled = true;
+ break;
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Seeking to exactly `MediaDuration` triggers the existing `MediaEnded` handler, which immediately resets `MediaPosition` to `0L`; consequently, the `End` hotkey does not leave playback at the end of the video.
**Triggers:** When `End` is pressed on a loaded video and the media backend raises `MediaEnded` after positioning at its duration.
**Suggested fix:** Seek to the last valid position before the duration, or suppress the end-reset behavior for an explicit end seek.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: QuickLook.Plugin/QuickLook.Plugin.VideoViewer/ViewerPanel.xaml.cs:319, QuickLook.Plugin/QuickLook.Plugin.VideoViewer/ViewerPanel.xaml.cs:316
| e.Handled = true; | ||
| break; | ||
|
|
||
| case Key.OemPlus or Key.Add when modifiers == ModifierKeys.None: |
There was a problem hiding this comment.
issue (bug_risk): The + playback-speed shortcut does not work on a standard keyboard because + is produced as Shift+OemPlus, while this branch only matches OemPlus when ModifierKeys.None is active.
Triggers: When the user presses the standard keyboard + key rather than NumPad Add.
Suggested fix: Accept ModifierKeys.Shift for Key.OemPlus (while retaining the unmodified Key.Add case for the numeric keypad).
Suggested change
| case Key.OemPlus or Key.Add when modifiers == ModifierKeys.None: | |
| case Key.OemPlus when modifiers == ModifierKeys.Shift: | |
| case Key.Add when modifiers == ModifierKeys.None: |
Comment on lines
+315
to
+316
| SeekTo(mediaElement.MediaDuration); | ||
| e.Handled = true; |
There was a problem hiding this comment.
issue (bug_risk): Seeking to exactly MediaDuration triggers the existing MediaEnded handler, which immediately resets MediaPosition to 0L; consequently, the End hotkey does not leave playback at the end of the video.
Triggers: When End is pressed on a loaded video and the media backend raises MediaEnded after positioning at its duration.
Suggested fix: Seek to the last valid position before the duration, or suppress the end-reset behavior for an explicit end seek.
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
1 participant
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.
Summary
This PR adds two features to the
QuickLook.Plugin.VideoViewerplugin:Playback speed control
1x), styled to match the existingHardwareAccelerationButtonStyle.0.25x, 0.5x, 0.75x, 1x, 1.25x, 1.5x, 1.75x, 2x.+/-to step through presets,0to reset to1x.SettingHelperand restored the next time a video is opened.MediaUriElement.SpeedRatio(already exposed byWPFMediaKit), no changes to native/DirectShow code required.Timeline seek hotkeys
Shift+Left/Right— seek 5 seconds backward/forward.Ctrl+Left/Right— seek 30 seconds backward/forward.Home/End— jump to the start/end of the video.[0, MediaDuration]and works withMediaPosition/MediaDuration(100ns ticks), consistent with the existing seeking API.Why
QuickLook's video preview currently has no way to change playback speed or jump around the timeline without dragging the seek bar with the mouse — both are common expectations for a media preview tool.
Implementation notes
QuickLook.Plugin.VideoViewer(ViewerPanel.xaml,ViewerPanel.xaml.cs,Styles.xaml,Translations.config).PreviewKeyDownis used for the new hotkeys and does not interfere with QuickLook's existing global hotkey dispatcher (KeystrokeDispatcher), since it only handles keys not already used elsewhere in the viewer.BTN_Speedtranslation strings forenandru-RU; other locales fall back to English.Testing
Summary by Sourcery
Add playback speed control and keyboard-based timeline navigation to the video viewer.
New Features:
Enhancements: