Mirrors/PowerToys
1
0
Fork 0
mirror of https://github.com/microsoft/PowerToys.git synced 2025-12-16 03:37:59 +01:00
Code Issues Packages Projects Releases Wiki Activity

[Peek] Fix media sources remaining locked when Peek window is closed (#42055)

Browse Source 
<!-- Enter a brief description/summary of your PR here. What does it
fix/what does it change/how was it tested (even manually, if necessary)?
-->
## Summary of the Pull Request

Fixes an issue where audio and video files would remain locked by Peek
after the user closed the preview window. This could prevent the
ejecting of removable storage.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist

- [x] Closes: #41883
- [ ] **Communication:** I've discussed this with core contributors
already. If the work hasn't been agreed, this work might be rejected
- [ ] **Tests:** Added/updated and all pass
- [ ] **Localization:** All end-user-facing strings can be localized
- [ ] **Dev docs:** Added/updated
- [ ] **New binaries:** Added on the required places
- [ ] [JSON for
signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json)
for new binaries
- [ ] [WXS for
installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs)
for new binaries and localization folder
- [ ] [YML for CI
pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml)
for new test projects
- [ ] [YML for signed
pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml)
- [ ] **Documentation updated:** If checked, please file a pull request
on [our docs
repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys)
and link it here: #xxx

<!-- Provide a more detailed description of the PR, other things fixed,
or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments

When the Peek window is closed, it is hiden rather than destroyed,
allowing for quick reactivation. The cleanup process is triggered by
`MainWindowViewModel.Uninitialize()`, which eventually causes the active
`Previewer` instance in `FilePreviewer.xaml.cs` to be disposed of (via
`OnItemPropertyChanged()`).

The root cause of the bug is that neither `VideoPreviewer` nor
`AudioPreviewer` implemented `IDisposable` sufficiently to cover clean
up of the media file. The result was that the `MediaSource` object,
which holds the handle to the open media file, was not disposed
deterministically. This left the file locked until the GC eventually
finalised it.

### Solution
To fix this, I've implemented `IDisposable` on `AudioPreviewer` where it
was absent, and expanded `Dispose()` on `VideoPreviewer` to clean up the
media object. A new `Unload()` method on each previewer handles explicit
clean up of the media sources. The `Dispose()` method calls `Unload()`
to ensure the resources are released reliably when the previewer is
destroyed.

`Unload()` is deliberately public, as I'd like to expand upon its use in
a future refactoring of the previewer switching logic, allowing for
reusing an active previewer instead of creating a new one every time the
user navigates to a new file.

### AudioPreviewer refinements
I made a few updates to `AudioPreviewer` to make the lifecycle of the
media source more predictable and improve the error handling:
- The `Preview` observable property is now a nullable
`AudioPreviewData?`. This more accurately represents the state (data is
either present or not), and allows for the removal of the creation of an
empty object in the constructor.
- `thumbnailTask`'s success is no longer regarded as an error condition
in `LoadPreviewAsync`. It is reasonable to let the user play their audio
file even if the thumbnail cannot be shown.
- An error when loading the source data or metadata results in the new
`Unload()` method being called, correctly releasing the file, protecting
against the file lock issue even if the media file was corrupt and could
only be partially constructed.
- Null checks for `Preview`, obvs.

### Scope
I could repro the original issue on audio and video files, so this PR is
focused on improvements to `AudioPreviewer` and `VideoPreviewer`. I
wasn't able to reproduce the file locking problem with the
`WebBrowserPreviewer` for its built-in supported types (HTML, Markdown
and PDF). I found some additional issues with the previewer, however,
but they will be covered in a future PR.

<!-- Describe how you validated the behavior. Add automated tests
wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
Tests were performed with the following steps:

1. Place the audio/video file to test on a USB stick.
2. Open the file with Peek and play some of the media.
3. While previewing the media, close the Peek window.
4. Attempt to eject the USB stick via the systray.
- Before: the eject fails and Windows pops up the "File in use" dialog.
    - After: the drive is successfully removed.

I tested an MP4 video file and an MP3 audio track to confirm the fix.

I confirmed that other previewers were unaffected by previewing: JPEG
image files, a PDF, a TXT file, a Markdown file, an HTML file and a
folder. All items were successfully previewed.
This commit is contained in:
Dave Rayment
2025-11-06 09:31:48 +00:00
committed by GitHub
parent e7de5c7b8d
commit 5c96cea31f
2 changed files with 59 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
src/modules/peek/Peek.FilePreviewer/Previewers/MediaPreviewer/AudioPreviewer.cs
View File

@@ -24,13 +24,15 @@ using Windows.Storage;
namespace Peek.FilePreviewer.Previewers.MediaPreviewer
{
public partial class AudioPreviewer : ObservableObject, IAudioPreviewer
public partial class AudioPreviewer : ObservableObject, IDisposable, IAudioPreviewer
{
private MediaSource? _mediaSource;
[ObservableProperty]
private PreviewState _state;
[ObservableProperty]
private AudioPreviewData _preview;
private AudioPreviewData? _preview;
private IFileSystemItem Item { get; }
@@ -40,7 +42,6 @@ namespace Peek.FilePreviewer.Previewers.MediaPreviewer
{
Item = file;
Dispatcher = DispatcherQueue.GetForCurrentThread();
Preview = new AudioPreviewData();
}
public async Task CopyAsync()
@@ -63,19 +64,23 @@ namespace Peek.FilePreviewer.Previewers.MediaPreviewer
{
State = PreviewState.Loading;
Preview = new AudioPreviewData();
var thumbnailTask = LoadThumbnailAsync(cancellationToken);
var sourceTask = LoadSourceAsync(cancellationToken);
var metadataTask = LoadMetadataAsync(cancellationToken);
await Task.WhenAll(thumbnailTask, sourceTask, metadataTask);
if (!thumbnailTask.Result || !sourceTask.Result || !metadataTask.Result)
if (sourceTask.Result && metadataTask.Result)
{
State = PreviewState.Error;
State = PreviewState.Loaded;
}
else
{
State = PreviewState.Loaded;
// Release all resources on error.
Unload();
State = PreviewState.Error;
}
}
@@ -88,12 +93,15 @@ namespace Peek.FilePreviewer.Previewers.MediaPreviewer
{
cancellationToken.ThrowIfCancellationRequested();
if (Preview != null)
{
var thumbnail = await ThumbnailHelper.GetThumbnailAsync(Item.Path, cancellationToken)
?? await ThumbnailHelper.GetIconAsync(Item.Path, cancellationToken);
cancellationToken.ThrowIfCancellationRequested();
Preview.Thumbnail = thumbnail ?? new SvgImageSource(new Uri("ms-appx:///Assets/Peek/DefaultFileIcon.svg"));
}
});
});
}
@@ -110,7 +118,11 @@ namespace Peek.FilePreviewer.Previewers.MediaPreviewer
{
cancellationToken.ThrowIfCancellationRequested();
Preview.MediaSource = MediaSource.CreateFromStorageFile(storageFile);
if (Preview != null)
{
_mediaSource = MediaSource.CreateFromStorageFile(storageFile);
Preview.MediaSource = _mediaSource;
}
});
});
}
@@ -123,6 +135,11 @@ namespace Peek.FilePreviewer.Previewers.MediaPreviewer
await Dispatcher.RunOnUiThread(() =>
{
if (Preview == null)
{
return;
}
cancellationToken.ThrowIfCancellationRequested();
Preview.Title = PropertyStoreHelper.TryGetStringProperty(Item.Path, PropertyKey.MusicTitle)
?? Item.Name[..^Item.Extension.Length];
@@ -160,6 +177,22 @@ namespace Peek.FilePreviewer.Previewers.MediaPreviewer
return _supportedFileTypes.Contains(item.Extension);
}
public void Dispose()
{
Unload();
GC.SuppressFinalize(this);
}
/// <summary>
/// Explicitly unloads the preview and releases file resources.
/// </summary>
public void Unload()
{
_mediaSource?.Dispose();
_mediaSource = null;
Preview = null;
}
private static readonly HashSet<string> _supportedFileTypes = new()
{
".aac",

16
src/modules/peek/Peek.FilePreviewer/Previewers/MediaPreviewer/VideoPreviewer.cs
View File

@@ -25,6 +25,8 @@ namespace Peek.FilePreviewer.Previewers
{
public partial class VideoPreviewer : ObservableObject, IVideoPreviewer, IDisposable
{
private MediaSource? _mediaSource;
[ObservableProperty]
private MediaSource? preview;
@@ -56,6 +58,7 @@ namespace Peek.FilePreviewer.Previewers
public void Dispose()
{
Unload();
GC.SuppressFinalize(this);
}
@@ -145,7 +148,8 @@ namespace Peek.FilePreviewer.Previewers
MissingCodecName = missingCodecName;
}
Preview = MediaSource.CreateFromStorageFile(storageFile);
_mediaSource = MediaSource.CreateFromStorageFile(storageFile);
Preview = _mediaSource;
});
});
}
@@ -155,6 +159,16 @@ namespace Peek.FilePreviewer.Previewers
return !(VideoTask?.Result ?? true);
}
/// <summary>
/// Explicitly unloads the preview and releases file resources.
/// </summary>
public void Unload()
{
_mediaSource?.Dispose();
_mediaSource = null;
Preview = null;
}
private static readonly HashSet<string> _supportedFileTypes = new()
{
".mp4", ".3g2", ".3gp", ".3gp2", ".3gpp", ".asf", ".avi", ".m2t", ".m2ts",