Conversation
The portal picker talks to xdg-desktop-portal over D-Bus, which only exists on Linux. FilePicker was already an abstract interface with a single implementation behind it, so this adds a second one built on QFileDialog and lets the .pro files choose per platform. Windows and macOS both get their own native dialog out of it. The one thing that does not carry over is the portal's "choices" combo, which is how the export dialog asks for a quality. Native save dialogs have no field to put that in, so the choices ride in the "Save as type" combo they already show -- still one dialog, still one pick. QFileDialog is a widget, so the non-portal build links QtWidgets and constructs a QApplication. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Exports encode to a sibling temp file and rename it over the target only once ffmpeg succeeds, so a failed cut leaves any existing file alone. POSIX rename() replaces the destination; the Windows CRT's refuses when it exists, which is precisely the case this path is written for -- so on Windows every re-export over a previous cut failed with "Could not write the exported file" and left the .omacut-part.mp4 behind. MoveFileExW with MOVEFILE_REPLACE_EXISTING does the replacement in one step, and takes the path as UTF-16 rather than through QFile::encodeName. Covered by the existing exportClipCanReplaceSourceFile and failedExportPreservesExistingFile tests, which only exercised the POSIX path before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A distro package declares ffmpeg as a dependency and finds it on the PATH. A self-contained build has no package manager to lean on and ships ffmpeg beside the executable instead, where findExecutable would never look -- it only searches the PATH. Check the application directory first, so a bundled pair is always the one that gets used, and keep the PATH as the fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three things stopped it on Windows, none of them the code under test: PATH entries were joined with ':', so the directory holding the deliberately-broken ffmpeg became part of one nonsense entry and was never searched. The real ffmpeg ran instead, the export succeeded, and the two tests waiting for a failure signal waited forever. The broken ffmpeg was a shebang script named "ffmpeg". Windows finds executables by extension and starts them with CreateProcess, which knows nothing about shebangs, so it was neither found nor started. An "ffmpeg.exe" holding no valid executable image fails to start for the reason the test actually wants. And exportStartFailureClearsBusy checked that the export is briefly busy after a failed start, which holds only where a failed exec is reported asynchronously. CreateProcess fails inline, so the cleanup has already run by the time exportClip() returns. The assertions that matter still run everywhere. The build of the test binaries needed adjusting on both platforms too: qmake produces a GUI-subsystem executable on Windows, leaving QtTest with no stdout to print results to, and an .app bundle on macOS, which bin/test looks straight past. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Thanks for omacut — it's lovely, and I wanted it on my Windows machine.
It turned out to need very little, almost entirely because
FilePickerwasalready an abstraction with the portal sitting behind it. This adds a second
implementation on
QFileDialogand lets the.profiles choose one perplatform. Linux is untouched by design: the portal path is unchanged, and every
new branch sits behind
OMACUT_PORTAL_FILE_PICKERorQ_OS_WIN. Windows andmacOS both get their own native dialog out of it.
The one real compromise
The portal's
choicescombo is how the export dialog asks for a quality, andnative save dialogs have no field to put one in. So the choices ride in the
"Save as type" combo those dialogs already show — still one dialog, still one
pick, but it is a genuine difference in feel and you may well not like it.
exportFiltersandscaleHeightForFilterare static and pure, so the pairingis covered without driving a modal dialog.
Three portability fixes
These came out of getting the suite green on Windows, and two of them are in the
tests rather than the app:
replaceWithTempusesstd::rename, which replaces the destination on POSIXbut fails on Windows when it already exists — which is exactly the
re-export-over-a-previous-cut case the function exists for.
MoveFileExWdoesit in one step. Linux behaviour is unchanged.
PATHwith':'. On Windows that collapses into one nonsenseentry, so the deliberately-broken ffmpeg was never found, the real one ran,
the export succeeded, and the two tests waiting for a failure signal waited
forever.
ffmpeg. Windows looksexecutables up by extension and starts them with
CreateProcess, which has nonotion of a shebang, so it was neither found nor started.
ffmpeg::toolPathalso looks in the application directory before thePATH, soa self-contained build can ship ffmpeg beside the binary. A packaged install
still resolves through the
PATHexactly as before.Testing
./bin/buildand./bin/test.bin/testneeded one nudge to work on macOS at all — qmake produces an.appbundle there, and the script ends in
./backend_tests, so it failed with "Nosuch file or directory" after a perfectly good build.
Worth pushing back on
The non-portal build links
QtWidgetsand constructs aQApplication, becauseQFileDialogis a widget. If you'd rather not take that dependency, aQtQuick.DialogsFileDialogavoids widgets but inverts the control flow thatFilePickerhas today. Happy to redo it that way, split this up, or drop anypiece of it — and equally happy if Windows just isn't somewhere you want to go.
Windows packaging (bundled ffmpeg, CI, archive layout) deliberately stays out of
this PR; it lives in https://github.com/deathy/omacut-windows.