Windows Arm64 support #1872
Open
Windows Arm64 support #1872
Conversation
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
Reviewer's GuideAdds Windows Arm64 support across the QuickLook core app, native bridge, and key plugins (especially the VideoViewer) by detecting Arm64 at runtime, routing to new Arm64 native DLLs and media pipelines, and updating projects/solution to build against the new architecture and toolset. Sequence diagram for video preview on Arm64 vs x64sequenceDiagram
actor User
participant ViewerWindow
participant Plugin
participant ViewerPanel
participant MediaElementWPF
participant MediaElementUri
User->>ViewerWindow: Request preview for file path
ViewerWindow->>Plugin: View(path, context)
Plugin->>Plugin: isArm64 = RuntimeInformation.ProcessArchitecture == Architecture.Arm64
alt Arm64
Plugin->>ViewerPanel: LoadAndPlayWPF(path, mediaInfo)
ViewerPanel->>ViewerPanel: InitializeArm64()
ViewerPanel->>MediaElementWPF: Source = new Uri(path)
ViewerPanel->>MediaElementWPF: Play()
else x64 or x86
Plugin->>ViewerPanel: LoadAndPlay(path, mediaInfo)
ViewerPanel->>ViewerPanel: InitializeDefault()
ViewerPanel->>MediaElementUri: MediaUriPlayer.LAVFilterDirectory = LAVFilters-x64 or LAVFilters-x86
ViewerPanel->>MediaElementUri: Play()
end
Class diagram for ViewerPanel with Arm64-specific video pipelineclassDiagram
class ViewerPanel {
-ContextObject _context
-BitmapSource _coverArt
-DispatcherTimer _lyricTimer
-DispatcherTimer timer
-bool IsSeeked
-bool _isPlaying
-bool _wasPlaying
-bool _shouldLoop
-bool isArm64
+bool HasVideo
+bool IsPlaying
+bool ShouldLoop
+double LinearVolume
+ViewerPanel(ContextObject context)
-void InitializeDefault()
-void InitializeArm64()
-void Seek_Timer(object sender, EventArgs e)
-void Seek_Drag_Started(object sender, DragStartedEventArgs e)
-void Seek_Drag_Completed(object sender, DragCompletedEventArgs e)
-void Seek_Value_Changed(object sender, RoutedPropertyChangedEventArgs~double~ e)
-void MediaOpened(object o, RoutedEventArgs args)
-void MediaEnded(object sender, RoutedEventArgs e)
-void MediaFailed(object sender, MediaFailedEventArgs e)
-void UpdateMeta(string path, MediaInfoLib info)
-void ChangeVolume(double delta)
-void TogglePlayPause(object sender, EventArgs e)
-void ToggleShouldLoop(object sender, EventArgs e)
+void LoadAndPlay(string path, MediaInfoLib info)
+void LoadAndPlayWPF(string path, MediaInfoLib info)
+void Dispose()
}
class MediaElementUri {
+bool IsPlaying
+bool HasVideo
+long MediaPosition
+double Volume
+MediaUriPlayer MediaUriPlayer
+void Play()
+void Pause()
+void Close()
}
class MediaElementWPF {
+Uri Source
+bool HasVideo
+TimeSpan Position
+Duration NaturalDuration
+double Volume
+void Play()
+void Pause()
+void Close()
}
class MediaUriPlayer {
+string LAVFilterDirectory
+void Dispose()
}
class ContextObject {
+bool IsBusy
+object ViewerContent
+string Title
}
ViewerPanel --> MediaElementUri : uses mediaElement
ViewerPanel --> MediaElementWPF : uses mediaElementWPF
MediaElementUri --> MediaUriPlayer
ViewerPanel --> ContextObject
Class diagram for QuickLook native bridge with Arm64 DLLsclassDiagram
class App {
+string AppFullPath
+string AppPath
+bool Is64Bit
+bool IsArm64
+bool IsUWP
+bool IsWin11
+bool IsWin10
}
class QuickLookNative {
-void Init_32()
-void Init_64()
-void Init_arm64()
-FocusedWindowType GetFocusedWindowTypeNative_32()
-FocusedWindowType GetFocusedWindowTypeNative_64()
-FocusedWindowType GetFocusedWindowTypeNative_arm64()
-void GetCurrentSelectionNative_32(StringBuilder sb)
-void GetCurrentSelectionNative_64(StringBuilder sb)
-void GetCurrentSelectionNative_arm64(StringBuilder sb)
+void Init()
+FocusedWindowType GetFocusedWindowType()
+string GetCurrentSelection()
}
class FocusedWindowType {
}
App <.. QuickLookNative : uses IsArm64
QuickLookNative --> FocusedWindowType
Class diagram for VideoViewer Plugin and MediaInfo selectionclassDiagram
class Plugin {
-static MediaInfoLib _mediaInfo
-ViewerPanel _vp
-static bool isArm64
+int Priority
+Plugin()
+void View(string path, ContextObject context)
+void Cleanup()
}
class MediaInfoLib {
+MediaInfoLib(string path)
+string Get(StreamKind kind, int index, string parameter)
+string Option(string option, string value)
}
class StreamKind {
}
Plugin --> ViewerPanel
Plugin --> MediaInfoLib : selects MediaInfo-arm64 or x86/x64
MediaInfoLib --> StreamKind
Class diagram for new time converters in VideoViewerclassDiagram
class TimeToLongConverter {
+object Convert(object value, Type targetType, object parameter, CultureInfo culture)
+object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
}
class TimeToShortStringConverter {
+object Convert(object value, Type targetType, object parameter, CultureInfo culture)
+object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
}
class IValueConverter {
<<interface>>
+object Convert(object value, Type targetType, object parameter, CultureInfo culture)
+object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
}
class DependencyObject {
}
TimeToLongConverter ..|> IValueConverter
TimeToShortStringConverter ..|> IValueConverter
TimeToLongConverter --|> DependencyObject
TimeToShortStringConverter --|> DependencyObject
File-Level Changes
Possibly linked issues
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 4 issues, and left some high level feedback:
- In
InitializeArm64,buttonTimeWPF.Clickis wired twice (one referencingbuttonTimeWPF.Tagand onebuttonTime.Tag), which looks like a copy‑paste mistake and may be toggling the wrong control’s state; consider consolidating this into a single handler for the correct button. - The new ARM64 slider seek helpers (
Seek_Drag_Started,Seek_Drag_Completed,Seek_Value_Changed) are not hooked up tosliderProgressWPFevents, so the seek UX may not work as intended; wire these methods to the correspondingThumb/ValueChangedevents or remove them if not needed. - The change from
ContextObject.IsBlockedtoContextObject.IsBusyinViewerWindow.Actions.BeginShowalters the gating logic for showing the busy TextBlock; please double‑check that this is intentional and consistent with how these flags are used elsewhere.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `InitializeArm64`, `buttonTimeWPF.Click` is wired twice (one referencing `buttonTimeWPF.Tag` and one `buttonTime.Tag`), which looks like a copy‑paste mistake and may be toggling the wrong control’s state; consider consolidating this into a single handler for the correct button.
- The new ARM64 slider seek helpers (`Seek_Drag_Started`, `Seek_Drag_Completed`, `Seek_Value_Changed`) are not hooked up to `sliderProgressWPF` events, so the seek UX may not work as intended; wire these methods to the corresponding `Thumb`/`ValueChanged` events or remove them if not needed.
- The change from `ContextObject.IsBlocked` to `ContextObject.IsBusy` in `ViewerWindow.Actions.BeginShow` alters the gating logic for showing the busy TextBlock; please double‑check that this is intentional and consistent with how these flags are used elsewhere.
## Individual Comments
### Comment 1
<location> `QuickLook.Plugin/QuickLook.Plugin.VideoViewer/ViewerPanel.xaml.cs:129-138` </location>
<code_context>
+ mediaElementWPF.MediaOpened += MediaOpened;
+ mediaElementWPF.MediaEnded += MediaEnded;
+
+ buttonTimeWPF.Click += (_, _) => buttonTimeWPF.Tag = (string)buttonTimeWPF.Tag == "Time" ? "Length" : "Time";
+
+
+ timer = new DispatcherTimer
+ {
+ Interval = TimeSpan.FromSeconds(1)
+ };
+ timer.Tick += Seek_Timer;
+ timer.Start();
+
+ buttonTimeWPF.Click += (_, _) => buttonTime.Tag = (string)buttonTime.Tag == "Time" ? "Length" : "Time";
+
+ sliderProgressWPF.PreviewMouseDown += (_, e) =>
</code_context>
<issue_to_address>
**issue (bug_risk):** Duplicate click handler toggles both buttonTimeWPF and buttonTime tags, likely a copy-paste error.
The second `buttonTimeWPF.Click` handler updates `buttonTime.Tag`, unintentionally coupling the WPF button to the non-arm64 control and causing their states to diverge or alter a hidden control. Please remove or correct this handler so only the appropriate button’s `Tag` is changed.
</issue_to_address>
### Comment 2
<location> `QuickLook.Plugin/QuickLook.Plugin.VideoViewer/ViewerPanel.xaml.cs:210-218` </location>
<code_context>
if (value == _shouldLoop) return;
_shouldLoop = value;
OnPropertyChanged();
+ if (!IsPlaying)
+ {
+ IsPlaying = true;
+
+ if (isArm64) mediaElementWPF.Play(); else mediaElement.Play();
+ }
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Changing ShouldLoop now auto-starts playback, which is a surprising side effect.
The setter now forces `IsPlaying = true` and calls `Play()` whenever looping is enabled while not playing, whereas previously it didn’t alter playback state. This hidden side effect can break callers that only want to configure looping (e.g., set options before starting). Please either constrain this behavior to a clearly intended context (e.g., only when resuming previously playing media) or keep `ShouldLoop` as a pure configuration flag and let existing playback controls start playback.
```suggestion
if (value == _shouldLoop) return;
_shouldLoop = value;
OnPropertyChanged();
```
</issue_to_address>
### Comment 3
<location> `QuickLook/ViewerWindow.Actions.cs:204` </location>
<code_context>
}
- if (ContextObject.IsBlocked)
+ if (ContextObject.IsBusy)
{
ContextObject.ViewerContent = new System.Windows.Controls.TextBlock
</code_context>
<issue_to_address>
**issue (bug_risk):** Using IsBusy instead of IsBlocked changes the error handling semantics and may be incorrect.
Previously this branch ran only when `ContextObject.IsBlocked`, which suggests a persistent error or crash state. Changing it to `IsBusy` will show the "crashed plugin" UI while a plugin is just loading, masking a valid busy state. Unless the model semantics changed, this should likely remain `IsBlocked` (or explicitly combine `IsBusy`/`IsBlocked`) to keep error and loading states distinct.
</issue_to_address>
### Comment 4
<location> `QuickLook.Plugin/QuickLook.Plugin.TextViewer/Plugin.MoreMenu.cs:32-40` </location>
<code_context>
{
string translationFile = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Translations.config");
- string extension = Path.GetExtension(_currentPath).ToLower();
+ string extension = "";
+ try
+ {
+ extension = Path.GetExtension(_currentPath).ToLower();
+ } catch (System.Exception)
+ {
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Empty catch-all around Path.GetExtension hides the real problem; a targeted null/invalid-path check would be clearer.
Catching `System.Exception` and defaulting to an empty extension will hide real issues. The likely problem here is a null or invalid `_currentPath`. Prefer checking `_currentPath` for null/empty before calling `GetExtension`, or at most catching `ArgumentException`, so unexpected errors still surface.
```suggestion
string translationFile = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Translations.config");
string extension = string.Empty;
if (!string.IsNullOrEmpty(_currentPath))
{
try
{
extension = Path.GetExtension(_currentPath).ToLowerInvariant();
}
catch (ArgumentException)
{
// Invalid path format; leave extension as empty.
}
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+129
to
+138
| buttonTimeWPF.Click += (_, _) => buttonTimeWPF.Tag = (string)buttonTimeWPF.Tag == "Time" ? "Length" : "Time"; | ||
|
|
||
|
|
||
| timer = new DispatcherTimer | ||
| { | ||
| Interval = TimeSpan.FromSeconds(1) | ||
| }; | ||
| timer.Tick += Seek_Timer; | ||
| timer.Start(); | ||
|
|
There was a problem hiding this comment.
issue (bug_risk): Duplicate click handler toggles both buttonTimeWPF and buttonTime tags, likely a copy-paste error.
The second buttonTimeWPF.Click handler updates buttonTime.Tag, unintentionally coupling the WPF button to the non-arm64 control and causing their states to diverge or alter a hidden control. Please remove or correct this handler so only the appropriate button’s Tag is changed.
Comment on lines
210
to
+218
| if (value == _shouldLoop) return; | ||
| _shouldLoop = value; | ||
| OnPropertyChanged(); | ||
| if (!IsPlaying) | ||
| { | ||
| IsPlaying = true; | ||
|
|
||
| if (isArm64) mediaElementWPF.Play(); else mediaElement.Play(); | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): Changing ShouldLoop now auto-starts playback, which is a surprising side effect.
The setter now forces IsPlaying = true and calls Play() whenever looping is enabled while not playing, whereas previously it didn’t alter playback state. This hidden side effect can break callers that only want to configure looping (e.g., set options before starting). Please either constrain this behavior to a clearly intended context (e.g., only when resuming previously playing media) or keep ShouldLoop as a pure configuration flag and let existing playback controls start playback.
Suggested change
| if (value == _shouldLoop) return; | |
| _shouldLoop = value; | |
| OnPropertyChanged(); | |
| if (!IsPlaying) | |
| { | |
| IsPlaying = true; | |
| if (isArm64) mediaElementWPF.Play(); else mediaElement.Play(); | |
| } | |
| if (value == _shouldLoop) return; | |
| _shouldLoop = value; | |
| OnPropertyChanged(); |
| } | ||
|
|
||
| if (ContextObject.IsBlocked) | ||
| if (ContextObject.IsBusy) |
There was a problem hiding this comment.
issue (bug_risk): Using IsBusy instead of IsBlocked changes the error handling semantics and may be incorrect.
Previously this branch ran only when ContextObject.IsBlocked, which suggests a persistent error or crash state. Changing it to IsBusy will show the "crashed plugin" UI while a plugin is just loading, masking a valid busy state. Unless the model semantics changed, this should likely remain IsBlocked (or explicitly combine IsBusy/IsBlocked) to keep error and loading states distinct.
Comment on lines
32
to
+40
| string translationFile = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Translations.config"); | ||
| string extension = Path.GetExtension(_currentPath).ToLower(); | ||
| string extension = ""; | ||
| try | ||
| { | ||
| extension = Path.GetExtension(_currentPath).ToLower(); | ||
| } catch (System.Exception) | ||
| { | ||
| } | ||
|
|
There was a problem hiding this comment.
suggestion (bug_risk): Empty catch-all around Path.GetExtension hides the real problem; a targeted null/invalid-path check would be clearer.
Catching System.Exception and defaulting to an empty extension will hide real issues. The likely problem here is a null or invalid _currentPath. Prefer checking _currentPath for null/empty before calling GetExtension, or at most catching ArgumentException, so unexpected errors still surface.
Suggested change
| string translationFile = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Translations.config"); | |
| string extension = Path.GetExtension(_currentPath).ToLower(); | |
| string extension = ""; | |
| try | |
| { | |
| extension = Path.GetExtension(_currentPath).ToLower(); | |
| } catch (System.Exception) | |
| { | |
| } | |
| string translationFile = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Translations.config"); | |
| string extension = string.Empty; | |
| if (!string.IsNullOrEmpty(_currentPath)) | |
| { | |
| try | |
| { | |
| extension = Path.GetExtension(_currentPath).ToLowerInvariant(); | |
| } | |
| catch (ArgumentException) | |
| { | |
| // Invalid path format; leave extension as empty. | |
| } | |
| } |
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.
PR Checklist
Brief Description of Changes
Please briefly describe the main changes in this PR:
Build on Visual Studio Community 2026 (v145)
.net Framework 4.8.1
Related Issue (if any)
Please provide related issue numbers:
wix doesn't work on v145
Additional Notes
Add any extra notes here:
video plugin uses native video, doesn't use lavfilters
Summary by Sourcery
Add Windows Arm64 runtime support across the core app and video viewer plugin, including architecture detection and native integrations.
New Features:
Bug Fixes:
Enhancements:
Build: