implemented export modified video issue #337 #339

mmulet · 2025-06-27T17:03:14Z

Fixes #337

save the modified video (trimming and cropping), so Gifski could also be used to cropping video. I'm thinking a "Export as Video…" in the "File" menu.
Export original audio

I implemented export of the original video, let me know what you think of the UI. Meanwhile, I will work on inserting the original audio track into the export.

mmulet · 2025-06-27T22:38:11Z

Added sound export support (works with trimming and speed changes)
Fix: a bug where the video preview would flash black when focus changed between export windows.
Fix: Can have multiple export windows open at the same time now.
Fix: Now, it only deleted the temp video when the window closes, not on save (wasn't really a bug, I just don't know why I did it that way).

Bug: I found a bug where if you change the speed of the video, TrimmingAVPlayer.timeRangeDidChange will be called with the full range of the video instead of the trimmed range.
So, if the time range is not full, and you change the speed, then the export time range will export the entire video instead of trimmed range.
I don't think this a bug from this pull request, it seems to be a bug in TrimmingAVPlayer itself. Is there a reason why it should be like this?

Nevermind, I went to take a video of this bug, and now I can't reproduce it.

sindresorhus · 2025-07-07T00:07:55Z

The video export is unlikely to take a long time, since most people use it for GIFs. So I think we can just show a sheet with a system spinner and cancel button if the video is longer than 20 seconds. And then afterwards, just immediately show the save dialog. One less click. And we don't really need the preview as the user can already preview in the main Gifski UI.

sindresorhus · 2025-07-07T00:23:59Z

You also need to add MP4 to NSExportableTypes in Info.plist.

Gifski/App.swift

Gifski/ExportModifiedVideo.swift

Gifski/Utilities.swift

mmulet · 2025-07-07T18:11:41Z

The video export is unlikely to take a long time, since most people use it for GIFs. So I think we can just show a sheet with a system spinner and cancel button if the video is longer than 20 seconds. And then afterwards, just immediately show the save dialog. One less click. And we don't really need the preview as the user can already preview in the main Gifski UI.
Add MP4 to NSExportableTypes in Info.plist.
Command+E shortcut to export modified video.
.mov -> .mp4
restore canBeginTrimming comment in Utilities.swift
Sorry, this was a mistake, shouldn't have been in the commit.
translatedBy(point: -> translated(by:
taskGroupMap -> concurrentMap
ClosedRange where Bound == Double -> ClosedRange<Double>

Gifski/EditScreen.swift

Gifski/ExportModifiedVideo.swift

sindresorhus · 2025-07-08T12:32:36Z

The audio preservation is complicating this a lot, when it's not directly needed for Gifski. Maybe we should just drop it? We could show an alert once, the first time the user tries to export a video with audio tracks.

sindresorhus · 2025-07-08T12:32:50Z

Also try to clean up and simplify the code more.

mmulet · 2025-07-08T14:40:57Z

The audio preservation is complicating this a lot, when it's not directly needed for Gifski. Maybe we should just drop it? We could show an alert once, the first time the user tries to export a video with audio tracks.

I agree.

Also try to clean up and simplify the code more.

Yeah, I'll take out the audio and simplify the PR a bunch.

… export_video2

mmulet · 2025-07-08T19:57:32Z

Remove audio
Remove unused code
Simplify code
Simplified it quite a bit.
Show error with Alert if the export fails instead of in the sheet.
Check error for really long text
Moved to appState.error
Drop cancellation
Move the fileExporter modifier out of the sheet
Remove unnecessary FileManager.removeItem
The setter is empty, meaning when the file export sheet is dismissed (e.g., user cancels), no state is updated.

mmulet · 2025-07-08T19:59:11Z

warning on export if audio track is present.

mmulet · 2025-07-09T15:58:23Z

Fix: If another alert (like bounce warning) occurs when you activate Save Modified Video, the filexporter won't show and the state will be stuck on .exported. This fixes that by reassigning the state to force a swiftUI draw and bring up the file exporter.

Gifski/ExportModifiedVideo.swift

Gifski/GIFGenerator.swift

Gifski/Utilities.swift

Gifski/ExportModifiedVideo.swift

Gifski/GIFGenerator.swift

mmulet · 2025-07-21T17:45:06Z

ExportModifiedVideoView.State -> ExportModifiedVideoState
GifGenerator.trackSize -> GifGenerator.trackDimensions
Style fixes like \s*/ -> */, else { -> else \n {
AVAsset.VideoMetadata.originalVideoHasAudio -> AVAsset.VideoMetadata.hasAudio
Export video Limitation -> Export Video Limitation
ExportModifiedVideoState.isExporting getters for isProgressSheetPresented, etc
Use AI to check for typos, bugs, and potential improvements for all the code in this pull request.
exported(..) -> finished(...)
GifGenerator.dimensionsAsSize -> GifGenerator.dimensionsAsCGSize

sindresorhus · 2025-08-08T18:59:57Z

#339 (comment) is not done.

mmulet · 2025-08-10T21:15:47Z

My bad! I must have missed somethings. I will look at it tomorrow.

…

On Fri, Aug 8, 2025, at 2:00 PM, Sindre Sorhus wrote: *sindresorhus* left a comment (sindresorhus/Gifski#339) <#339 (comment)> #339 (comment) <#339 (comment)> is not done. — Reply to this email directly, view it on GitHub <#339 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABRVPINIL6ZUMMQUQTHXUQD3MTXUHAVCNFSM6AAAAACAJP6BVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNRZGAZDEOBVGU>. You are receiving this because you authored the thread.Message ID: ***@***.***>

mmulet · 2025-08-11T17:51:01Z

Now it only shows the progress when the video is over 20 seconds long.

What else am I missing? It seems to work to me.

sindresorhus · 2025-08-12T00:46:57Z

That's not what the link points to. It's:

This is an unrelated independent state and should not be here.

mmulet · 2025-08-12T14:57:30Z

That's not what the link points to. It's:

This is an unrelated independent state and should not be here.

Ah, I see now. I went and moved the ExportVideoState to its own file.

sindresorhus · 2025-08-12T15:25:58Z

No, that's not it. The comment is on the case audioWarning line. And I meant that that case is not related to the state and should be a independent boolean state in the view itself, not part of this enum.

mmulet · 2025-08-12T15:28:03Z

No, that's not it. The comment is on the case audioWarning line. And I meant that that case is not related to the state and should be a independent boolean state in the view itself, not part of this enum.

Oh, that makes a lot more sense. Sorry about the confusion. I will fix it now.

mmulet · 2025-08-12T21:29:01Z

Move audioWarning out of ExportModifiedVideoState

sindresorhus · 2025-08-14T21:11:36Z

If I trim a video, and then change the speed, the saved video does not seem to respect the trim.

The speed also seems to be additive, so if I drag it to faster multiple times, it just gets faster.

Gifski/ExportModifiedVideoState.swift

Gifski/GIFGenerator.swift

Gifski/EditScreen.swift

Gifski/ExportModifiedVideo.swift

mmulet · 2025-08-19T01:24:17Z

Fix: If I trim a video, and then change the speed, the saved video does not seem to respect the trim.
This was a bug in AVTrimmingPlayerView, not from this pr, but I fixed it anyways.
Remove leftovers exportModifiedVideoState.swift file
GifGenerator return value fix
.toTimeInterval > 20 -> _ > .secondons(20)
remove leftover case
Use preferredTransform in ExportModifiedVideo to correctly handle videos with metadata that they are rotated.

Support for preferredTransform

preferredTransform turned out to be a can of worms, because almost nothing else worked with it! I had to make changes to preview, crop, intents, and normal gif export to support it.

Couldn't reproduce

Fix: The speed also seems to be additive, so if I drag it to faster multiple times, it just gets faster.
I could not reproduce this bug. I've attached a video of me trying to change the speed a lot and each time it exports correctly?

mmulet · 2025-08-19T01:39:07Z

exports just fine:

screen1.mp4

and I don't see the problem when scrubbing the speed while playing (although it does look like sometimes it does not change the play speed, which is a separate issue that I should look into*):

screen2.mp4

* I did look into it a bit. It's hard to diagnose because

It doesn't always happen. It works most of the time, and I'm still having trouble triggering it reliably.
When it does happen, neither setSpeed or or the on receive get an update.

.onReceive(Defaults.publisher(.outputSpeed, options: []).removeDuplicates().debounce(for: .seconds(0.4), scheduler: DispatchQueue.main)) { _ in
			Task {
				await setSpeed()
			}
		}

I'm out of time today to figure out what the problem is.

sindresorhus · 2025-09-29T16:19:58Z

Fix: The speed also seems to be additive, so if I drag it to faster multiple times, it just gets faster.

One way to reproduce each time is to set speed to 4x, then relaunch Gifski and then drag to 1x. It then doesn't play as 1x.

However, in the latest commits, I noticed the speed slider doesn't have any effect at all.

sindresorhus · 2025-09-30T02:02:55Z

Gifski/Components/TrimmingAVPlayer.swift

+		guard let timeRange,
+			  let currentItemDurationRange else {


Code style. Applies everywhere.

Suggested change

guard let timeRange,

let currentItemDurationRange else {

guard

let timeRange,

let currentItemDurationRange

else {

Sorry about this one. I was a bit confused because the alignment did look good on both, xcode and vscode (see below). But, I turned on whitespace and it looks like it was a interplay between tabs and spaces that the Github file viewer doesn't like. I'll switch everything over to pure tabs.

Gifski/Components/TrimmingAVPlayer.swift

sindresorhus · 2025-09-30T02:03:38Z

Crop API typos.
unormalziedCropFor → unnormalizedCropRect, prefferedSize → preferredSize, origninalSize → originalSize.
Wrong math for crop transform.
CGRect(origin:.zero,size:preferredSize).applying(trackPreferredTransform.inverted()).size to derive “original size” is bogus for 90° rotations (you get the bounding box, not the pre-rotated size). Don’t transform a size. Instead, unnormalize in original pixel space, then apply the preferred transform to the rect:
```
let norm = crop ?? .initialCropRect
let base = norm.unnormalize(forDimensions: preferredSize) // original pixel space
let rectInPreferred = trackPreferredTransform.map { base.applying($0) } ?? base
return rectInPreferred
```
Replace your unormalziedCropFor(...) body with the above logic.
croppedImage guard + logic mismatch.
You early-return when crop == nil, but unormalziedCropFor internally falls back to .initialCropRect anyway. Pick one rule: either (a) return the image when crop == nil and never use .initialCropRect internally, or (b) always crop using .initialCropRect when crop == nil. Right now it’s inconsistent. I’d do (a).
Time-range handoff bug on speed change.
In onNewDurationRange, when there’s no previous timeRange/currentItemDurationRange, you only call timeRangeDidChange?, but you don’t update self.timeRange. Set it so downstream code sees the correct range:
```
guard let timeRange, let currentItemDurationRange else {
	self.timeRange = newItemDurationRange
	timeRangeDidChange?(newItemDurationRange)
	return
}
```
You gate the spinner with gifDuration(...), which can double with bounce, while MP4 export ignores bounce. Base it on the non-bounce segment length (the modified asset/timeRange), not the GIF duration.
Export session nit:
Set exportSession.shouldOptimizeForNetworkUse = true after creating the session. It’s harmless and improves quick-start playback.

And as always, try to find a way to simplify. There are a lot of changes here.

mmulet · 2025-09-30T16:02:28Z

I will take a look at these suggestions and make the necessary changes this weekend.

…port_video2

Co-authored-by: Sindre Sorhus <[email protected]>

… export_video2

mmulet · 2025-10-04T17:33:17Z

Done

convert time range... -> Convert time range...
Crop api typos: unormalziedCropFor → unnormalizedCropRect, prefferedSize → preferredSize, origninalSize → originalSize.
Wrong math for crop transform.
- WontFix: This fix is "bogus", 1. it doesn't work. 2. The code I wrote does work.
croppedImage guard + logic mismatch
Time-range handoff bug on speed change
Decide whether or not to show spinner based on video length without bounce
Add exportSession.shouldOptimizeForNetworkUse = true

mmulet · 2025-10-05T18:07:24Z

Add icon to Export modified video menu item
Style fixes
Simplify code
Fix speed slider
The problem was with the combine code. So instead of using combine, I used the debouncer from utilities.swift and implemented deduping manually.
Reproduce The speed also seems to be additive, so if I drag it to faster multiple times, it just gets faster.
After fixing the speed slider, I did as you said: set the speed to 4x, then quit Gifski, then re-open it and set the speed to 1x. It worked correctly, so I couldn't reproduce the bug. I attached a video, and as you can see, it's all working fine.

Screen.Recording.2025-10-05.at.1.43.41.PM.mov

mmulet added 3 commits June 27, 2025 11:59

implemented export modified video (without audio for now)

b0e2ca9

add app group back in

862ee4f

export audio, plus fixes

23d08fd

sindresorhus requested changes Jul 7, 2025

View reviewed changes

implemented requested changes

5487cf6

Update Utilities.swift

cc2c721

sindresorhus requested changes Jul 8, 2025

View reviewed changes

mmulet added 3 commits July 8, 2025 14:51

implemented requested changes

f41ebc5

Merge remote-tracking branch 'origin/main' into export_video2

51f8990

Merge branch 'export_video2' of https://github.com/mmulet/Gifski into…

0853bcc

… export_video2

mmulet requested a review from sindresorhus July 8, 2025 19:56

mmulet added 2 commits July 8, 2025 15:59

added the warning on export if the video has audio

5ff5f76

fix bug where it stop exports if you start when another alert was active

3a4124c

mmulet and others added 2 commits July 9, 2025 11:27

more simplified

3bfb5de

Update EditScreen.swift

2cf84de

sindresorhus requested changes Jul 20, 2025

View reviewed changes

implemtned requested changes

8fa4535

Show progress only when video is longer than 20 seconds

2640165

move Export Modified Video State out of Export Modified video

b17c902

move audioWarning out of ExportModifiedVideoState

dfc2e49

sindresorhus requested changes Aug 14, 2025

View reviewed changes

mmulet added 2 commits August 18, 2025 15:57

implemented requested changes

2117c67

added more support for prefferredTransform

d2e2928

Update App.swift

7907b7c

sindresorhus reviewed Sep 30, 2025

View reviewed changes

Gifski/Components/TrimmingAVPlayer.swift Outdated Show resolved Hide resolved

mmulet and others added 4 commits October 4, 2025 12:16

Merge branch 'main' of https://github.com/sindresorhus/Gifski into ex…

1bc794c

…port_video2

Update Gifski/Components/TrimmingAVPlayer.swift

4053aac

Co-authored-by: Sindre Sorhus <[email protected]>

part 1 of implementing requested changes

e9804d6

Merge branch 'export_video2' of https://github.com/mmulet/Gifski into…

bc7e2bb

… export_video2

mmulet added 3 commits October 5, 2025 13:51

Fix speed slider bug

c0b54f2

simplify

8bc78da

style fixes

4dea67b

Uh oh!

implemented export modified video issue #337 #339

Are you sure you want to change the base?

implemented export modified video issue #337 #339

Uh oh!

Conversation

mmulet commented Jun 27, 2025 • edited by sindresorhus Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmulet commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sindresorhus commented Jul 7, 2025

Uh oh!

sindresorhus commented Jul 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mmulet commented Jul 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sindresorhus commented Jul 8, 2025

Uh oh!

sindresorhus commented Jul 8, 2025

Uh oh!

mmulet commented Jul 8, 2025

Uh oh!

mmulet commented Jul 8, 2025

Uh oh!

mmulet commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmulet commented Jul 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mmulet commented Jul 21, 2025

Uh oh!

sindresorhus commented Aug 8, 2025

Uh oh!

mmulet commented Aug 10, 2025 via email

Uh oh!

mmulet commented Aug 11, 2025

Uh oh!

sindresorhus commented Aug 12, 2025

Uh oh!

mmulet commented Aug 12, 2025

Uh oh!

sindresorhus commented Aug 12, 2025

Uh oh!

mmulet commented Aug 12, 2025

Uh oh!

mmulet commented Aug 12, 2025

Uh oh!

sindresorhus commented Aug 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mmulet commented Jun 27, 2025 •

edited by sindresorhus

Loading

mmulet commented Jun 27, 2025 •

edited

Loading

mmulet commented Jul 8, 2025 •

edited

Loading

mmulet commented Aug 19, 2025 •

edited

Loading

mmulet commented Sep 30, 2025 •

edited

Loading

mmulet commented Oct 4, 2025 •

edited

Loading