search

sindresorhus / Gifski

🌈 Convert videos to high-quality GIFs on your Mac
https://sindresorhus.com/gifski
MIT License
7.69k stars 290 forks source link

The trim leading video frames feature fails sometimes #254

Open sindresorhus opened 2 years ago

sindresorhus commented 2 years ago

@codetheweb I have noticed that there are a bunch of Crashlytics (automated) reports about this feature not working: "Could not trim empty leading frames from video.". Any ideas?

AVAsset debug info ## Extension: mov Video codec: BGRA Audio codec: nil Duration: 25s Dimension: 376×805 Frame rate: 9.79 File size: 305.4 MB Is readable: true Is playable: true Is exportable: true Has protected content: false Track #1 ---- Type: Video Codec: nil Duration: 25s Dimensions: 376×805 Natural size: (376.0, 805.0) Frame rate: 9.79 Is playable: true Is decodable: true ----

AVAsset debug info ## Extension: nil Video codec: v210 Audio codec: nil Duration: 3s Dimension: 900×700 Frame rate: 29.97 File size: 160 MB Is readable: true Is playable: true Is exportable: true Has protected content: false Track #1 ---- Type: Video Codec: nil Duration: 3s Dimensions: 900×700 Natural size: (900.0, 700.0) Frame rate: 29.97 Is playable: true Is decodable: true ----

AVAsset debug info ## Extension: nil Video codec: v210 Audio codec: nil Duration: 5s Dimension: 900×700 Frame rate: 29.97 File size: 275.8 MB Is readable: true Is playable: true Is exportable: true Has protected content: false Track #1 ---- Type: Video Codec: nil Duration: 5s Dimensions: 900×700 Natural size: (900.0, 700.0) Frame rate: 29.97 Is playable: true Is decodable: true ----

AVAsset debug info ## Extension: nil Video codec: v210 Audio codec: nil Duration: 2s Dimension: 900×700 Frame rate: 29.97 File size: 109 MB Is readable: true Is playable: true Is exportable: true Has protected content: false Track #1 ---- Type: Video Codec: nil Duration: 2s Dimensions: 900×700 Natural size: (900.0, 700.0) Frame rate: 29.97 Is playable: true Is decodable: true ----

codetheweb commented 2 years ago

I was able to reproduce this by creating a v210 encoded .mov file:

https://user-images.githubusercontent.com/7410405/134365364-ab83c10e-b7ca-4510-8d92-a78209dba128.mov

I'll try and take a look later today.

codetheweb commented 2 years ago

It seems that every sampleBuffer.totalSampleSize == 0 (https://github.com/sindresorhus/Gifski/blob/5c966175e4b449c23b95cc962acc111294441e27/Gifski/Utilities.swift#L636) when scanning through the file.

This suggests that a different output format might be necessary, but I tried the ones listed and none of them worked. Also tried the actual format (kCVPixelFormatType_422YpCbCr10) and that didn't work either.

The sample file I uploaded earlier is so small that it's not immediately clear that it does actually have data, so here's a bigger / longer one that you can play back:

https://user-images.githubusercontent.com/7410405/134410449-bab3e28d-1a9b-413f-ac3a-557067282874.mov

Not really sure where to go from here, not a video encoding expert by any means.

I would be curious to find out the source of these v210 files, I can't imagine many consumer-oriented tools produce video in it since it's completely uncompressed.

sindresorhus commented 2 years ago

Thanks for investigating. This really looks like a macOS bug...

How about we do this?

sindresorhus commented 2 years ago

In the latest version, there are still some reported failures for this:

AVAsset debug info ## Extension: nil Video codec: Audio codec: nil Duration: 1m 30s Dimension: 1920×1080 Frame rate: 25 File size: 14.04 GB Is readable: true Is playable: true Is exportable: true Has protected content: false Track #1 ---- Type: Video Codec: Duration: 1m 30s Dimensions: 1920×1080 Natural size: (1920.0, 1080.0) Frame rate: 25 Is playable: true Is decodable: true ----

sindresorhus commented 2 years ago

Another case.

The trim error says:

AVAsset debug info ## Extension: nil Video codec: Audio codec: nil Duration: 1s Dimension: 620×860 Frame rate: 24 File size: 51.5 MB Is readable: true Is playable: true Is exportable: true Has protected content: false Track #1 ---- Type: Video Codec: Duration: 1s Dimensions: 620×860 Natural size: (620.0, 860.0) Frame rate: 24 Is playable: true Is decodable: true ----

However, the AVAsset debug info logging reports:

AVAsset debug info ## Extension: mov Video codec: Apple ProRes 4444 (ap4h) Audio codec: nil Duration: 4s Dimension: 620×860 Frame rate: 24 File size: 55.2 MB Is readable: true Is playable: true Is exportable: true Has protected content: false Track #1 ---- Type: Video Codec: Apple ProRes 4444 (ap4h) Duration: 4s Dimensions: 620×860 Natural size: (620.0, 860.0) Frame rate: 24 Is playable: true Is decodable: true ---- Track #2 ---- Type: Time code Codec: tmcd Duration: 4s Dimensions: nil Natural size: (0.0, 0.0) Frame rate: 0.25 Is playable: true Is decodable: true ----

So it seems like re-wrapping causes it to lose the codec info.

It's also weird that it fails on Apple ProRes 4444 (ap4h), which is a commonly used codec.

codetheweb commented 2 years ago

Strangely both the App Store version and my locally built version of Gifski crash when selecting any video file (including regular old h264 .mp4s) with the error:

Gifski[8973:1704424] [plugin] AddInstanceForFactory: No factory registered for id <CFUUID 0x603000340ba0> F8BB1C28-BAE8-11D6-9C31-00039315CD46
Gifski[8973:1704424]  HALC_ShellDriverPlugIn::Open: opening the plug-in failed, Error: 2003329396 (what)

The only thing that I can think of that's changed is I upgraded to Monterey a few days ago.

Is the app working for you on Monterey? If so I'll debug further, just want to check if it's an OS issue so I don't waste time.

sindresorhus commented 2 years ago

@codetheweb I have gotten many reports about the crash on Moneterey, but I have been unable to reproduce it myself... Could you try the latest App Store version v2.19.2? I tried some workarounds there.

sindresorhus commented 2 years ago

@codetheweb So it seems my potential fix did not work... I don't think that logging is related. I see spurious logging like that all the time. According to many crash reports, there's something in the SwiftUI parts that crashes, so either the estimated size thing or the speed slider. Would you be able to try to comment some stuff out in the SwiftUI parts and see if anything makes it not crash? I'm really lost on what the problem might be and I'm not able to reproduce either.

codetheweb commented 2 years ago

Was able to fix it. The error seems to be here:

https://github.com/sindresorhus/Gifski/blob/5990ce3e7a2b3baae8beefbc52701a3336b37e85/Gifski/EstimatedFileSize.swift#L113

Specifying either .controlSize() or .scaleEffect() works fine, but specifying both results in a crash. I'm gonna hazard a guess that this is a SwiftUI bug. 😛

For now I'd probably just set .controlSize(.mini).

sindresorhus commented 2 years ago

Thank you so much for finding a fix. Can you confirm that Gifski v2.19.3 fixes the issue?

codetheweb commented 2 years ago

v2.19.3 does seem to have fixed it for me. 👍

sindresorhus commented 2 years ago

Thanks for confirming.

codetheweb commented 2 years ago

Getting back to the issue at hand:

Not sure what's going on. I created a file that's as close to one of the logs above:

AVAsset debug info ## Extension: mov Video codec: Apple ProRes 4444 (ap4h) Audio codec: nil Duration: 4s Dimension: 620×860 Frame rate: 24 File size: 55.2 MB Is readable: true Is playable: true Is exportable: true Has protected content: false Track #1 ---- Type: Video Codec: Apple ProRes 4444 (ap4h) Duration: 4s Dimensions: 620×860 Natural size: (620.0, 860.0) Frame rate: 24 Is playable: true Is decodable: true ---- Track #2 ---- Type: Time code Codec: tmcd Duration: 4s Dimensions: nil Natural size: (0.0, 0.0) Frame rate: 0.25 Is playable: true Is decodable: true ----

as possible (my file):

AVAsset debug info2: ## AVAsset debug info ##\nExtension: nil\nVideo codec: Apple ProRes 4444 (ap4h)\nAudio codec: nil\nDuration: 4s\nDimension: 620×860\nFrame rate: 24\nFile size: 49.4 MB\nIs readable: true\nIs playable: true\nIs exportable: true\nHas protected content: false\n\nTrack #1\n----\nType: Video\nCodec: Apple ProRes 4444 (ap4h)\nDuration: 4s\nDimensions: 620×860\nNatural size: (620.0, 860.0)\nFrame rate: 24\nIs playable: true\nIs decodable: true\n----"

but it was handled just fine.

This is a wild guess, but maybe there's some subtle differences in AVAsset handling depending on whether the architecture is x86 / ARM? Are you able to see the arch in crash reports?

Thanks for the sponsor! :)

sindresorhus commented 2 years ago

This is a wild guess, but maybe there's some subtle differences in AVAsset handling depending on whether the architecture is x86 / ARM? Are you able to see the arch in crash reports?

Only x86 machines.

sindresorhus commented 2 years ago

Noticed a new codec that failed:

AVAsset debug info ## Extension: nil Video codec: 2vuy Audio codec: nil Duration: 3s. Dimension: 1920×1080 Frame rate: 23.98 File size: 356.7 MB Is readable: true Is playable: true Is exportable: true Has protected content: false Track #1 ---- Type: Video Codec: 2vuy Duration: 3s. Dimensions: 1920×1080 Natural size: (1920.0, 1080.0) Frame rate: 23.98 Is playable: true Is decodable: true ----

codetheweb commented 2 years ago

Given that we're not doing anything particularly fancy and this is happening across multiple codecs it seems to be a bug in AVFoundation in some capacity, but since I can't reproduce it I don't know where to begin investigating. The other thing I thought of today is we could try preloading the metadata, but I doubt that would make a difference since it should automatically be loaded synchronously.

Maybe instead of keeping a codec blacklist we should keep a whitelist (specifically, probably only codecs for iOS screen recordings)?

codetheweb commented 2 years ago

Note that I did recently get an ARM MacBook and that ProRes file that I generated earlier that worked on x86 no longer seems to work on ARM (just shows black frames and the message "Failed to get estimate" in the corner). Although the same file was also previously playable in QuickTime and even that isn't working anymore.

Probably not related if all crash reports are on x86 but weird behavior nonetheless.

sindresorhus commented 2 years ago

Maybe instead of keeping a codec blacklist we should keep a whitelist (specifically, probably only codecs for iOS screen recordings)?

Maybe at some point, but it's not a big problem as we still let continue. I have a small hope that someone will eventually email me a test case...

sindresorhus commented 2 years ago

The other thing I thought of today is we could try preloading the metadata, but I doubt that would make a difference since it should automatically be loaded synchronously.

Yeah, I doubt that's it.

sindresorhus commented 2 years ago

Some more thoughts. Wonder if we need to check https://developer.apple.com/documentation/coremedia/cmsamplebuffer/3242544-datareadiness

Or whether we need to wait for https://developer.apple.com/documentation/coremedia/cmsamplebuffer/3242541-databecameready

Also slightly relevant: https://stackoverflow.com/questions/29385893/buffer-size-of-cmsamplebufferref

codetheweb commented 2 years ago

Maybe? I can't find much documentation on .dataReadiness. There's also .makeDataReady(), but again it's not clear if it actually does anything or if it just changes a property.

Doing CMSampleBufferGetImageBuffer(sampleBuffer) (as shown in the SO post) always returns nil.