implemented export modified video issue #337

[mmulet]

Fixes #337

• [x] 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.
• [x] 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]

• [x] Added sound export support (works with trimming and speed changes)
• [x] Fix: a bug where the video preview would flash black when focus changed between export windows.
• [x] Fix: Can have multiple export windows open at the same time now.
• [x] 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]

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]

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

[mmulet]

• [x] 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.
• [x] Add MP4 to NSExportableTypes in Info.plist.
• [x] Command+E shortcut to export modified video.
• [x] .mov -> .mp4
• [x] restore canBeginTrimming comment in Utilities.swift
• Sorry, this was a mistake, shouldn't have been in the commit.
• [x] translatedBy(point: -> translated(by:
• [x] taskGroupMap -> concurrentMap
• [x] ClosedRange where Bound == Double -> ClosedRange<Double>

[sindresorhus]

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]

Also try to clean up and simplify the code more.

[mmulet]

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.

[mmulet]

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

[mmulet]

• [x] warning on export if audio track is present.

[mmulet]

• [x] 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.

[mmulet]

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

[sindresorhus]

https://github.com/sindresorhus/Gifski/pull/339#discussion_r2217805519 is not done.

[mmulet]

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) https://github.com/sindresorhus/Gifski/pull/339#issuecomment-3169022855 #339 (comment) https://github.com/sindresorhus/Gifski/pull/339#discussion_r2217805519 is not done.

— Reply to this email directly, view it on GitHub https://github.com/sindresorhus/Gifski/pull/339#issuecomment-3169022855, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRVPINIL6ZUMMQUQTHXUQD3MTXUHAVCNFSM6AAAAACAJP6BVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNRZGAZDEOBVGU. You are receiving this because you authored the thread.Message ID: @.***>

[mmulet]

• [x] 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]

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

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

[mmulet]

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]

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]

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]

• [x] Move audioWarning out of ExportModifiedVideoState

[sindresorhus]

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.

[mmulet]

• [x] 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.
• [x] Remove leftovers exportModifiedVideoState.swift file
• [x] GifGenerator return value fix
• [x] .toTimeInterval > 20 -> _ > .secondons(20)
• [x] remove leftover case
• [x] 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]

exports just fine:

https://github.com/user-attachments/assets/1cc11df1-441c-4387-87c7-46436c1de993

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*):

https://github.com/user-attachments/assets/b9f2bb26-c9ef-4927-bab2-f39a7c014798

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

1. It doesn't always happen. It works most of the time, and I'm still having trouble triggering it reliably.
2. 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]

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]

• 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]

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

[mmulet]

Done

• [x] convert time range... -> Convert time range...
• [x] 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.
• [x] croppedImage guard + logic mismatch
• [x] Time-range handoff bug on speed change
• [x] Decide whether or not to show spinner based on video length without bounce
• [x] Add exportSession.shouldOptimizeForNetworkUse = true

[mmulet]

• [x] Add icon to Export modified video menu item
• [x] Style fixes
• [x] Simplify code
• [x] 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.

https://github.com/user-attachments/assets/6fd2b751-563c-4b29-a6e3-e3b71f0d7bfd