Discussion #1

sindresorhus · 2018-02-01T10:51:00Z

I'm having a couple of problems with libgifski. I'm probably using it incorrectly, but I've been stuck on it.

Update the paths in https://github.com/sindresorhus/gifski-draft/blob/793d9289961524667d47bc9d18a675166a374149/Gifski/AppDelegate.swift#L21 to your paths before testing.

The produced GIF has the wrong colors:

I'm probably passing the image as bytes incorrectly, but I'm not sure what I'm doing wrong.

I also have problems when trying to produce more than 4 frames. It seems to never finish then. Try to uncomment https://github.com/sindresorhus/gifski-draft/blob/master/Gifski/AppDelegate.swift#L48 and you'll see.

Is there any way for me to use libgifski with GCD? I tried, and Rust threw an error.

Any other feedback welcome.

kornelski · 2018-02-01T12:02:10Z

The buffer looks like it's ABGR rather than BGRA. You may need to byte-swap the pixels.

gifski_write() must be called before gifski_end_adding_frames(). It buffers only up to 4 frames, and then waits for ability to write them to disk.

I'll make a PR

sindresorhus · 2018-02-01T12:06:12Z

gifski_write() must be called before gifski_end_adding_frames(). It buffers only up to 4 frames, and then waits for ability to write them to disk.

I was following the docs example which calls gifski_end_adding_frames() before gifski_write().

kornelski · 2018-02-01T12:22:52Z

I'm sorry, I forgot to publish the docs fix (now live)

kornelski · 2018-02-01T12:25:41Z

97d3655?w=1

sindresorhus · 2018-02-02T07:34:21Z

Ah, ok. That makes it clearer. Thanks. I'm still not sure how to do byte-swapping of the pixels though.

sindresorhus · 2018-02-02T07:35:46Z

From the docs:

The callback is called once per frame with NULL, and then once with non-null message on end.

It never returns a non-nil message:

Frame: 0
Frame: 1
Frame: 2
Frame: 3
Frame: 4
Frame: 5
Frame: 6
Frame: 7
isDone nil
Frame: 8
isDone nil
Frame: 9
isDone nil
Frame: 10
isDone nil
Frame: 11
isDone nil
Frame: 12
isDone nil
Frame: 13
isDone nil
Frame: 14
isDone nil
Frame: 15
isDone nil
Frame: 16
isDone nil
Frame: 17
isDone nil
Frame: 18
isDone nil
Frame: 19
isDone nil
Frame: 20
isDone nil
Frame: 21
isDone nil
Frame: 22
isDone nil
Frame: 23
isDone nil
Frame: 24
isDone nil
Frame: 25
isDone nil
Frame: 26
isDone nil
Frame: 27
isDone nil
Frame: 28
isDone nil
Frame: 29
isDone nil
Frame: 30
isDone nil
Frame: 31
isDone nil
Frame: 32
isDone nil
Frame: 33
isDone nil
Frame: 34
isDone nil
Frame: 35
isDone nil
Frame: 36
isDone nil
Frame: 37
isDone nil
Frame: 38
isDone nil
Frame: 39
isDone nil
Frame: 40
isDone nil
Frame: 41
isDone nil
Frame: 42
isDone nil
Frame: 43
isDone nil
Frame: 44
isDone nil
Frame: 45
isDone nil
Frame: 46
isDone nil
Frame: 47
isDone nil
Frame: 48
isDone nil
Frame: 49
isDone nil
Frame: 50
isDone nil
Frame: 51
isDone nil
Frame: 52
isDone nil
Frame: 53
isDone nil
Frame: 54
isDone nil
Frame: 55
Frame: 56
isDone nil
Frame: 57
Frame: 58
Frame: 59
isDone nil
Frame: 60
Frame: 61
isDone nil
Frame: 62
isDone nil
isDone nil
isDone nil
isDone nil
isDone nil
Frame: 63
isDone nil
Frame: 64
isDone nil
Frame: 65
isDone nil
Frame: 66
isDone nil
Frame: 67
isDone nil
Frame: 68
isDone nil
Frame: 69
isDone nil
Frame: 70
isDone nil
Frame: 71
isDone nil
Frame: 72
isDone nil
Frame: 73
isDone nil
Frame: 74
isDone nil
Frame: 75
isDone nil
Frame: 76
isDone nil
Frame: 77
isDone nil
Frame: 78
isDone nil
Frame: 79
isDone nil
Frame: 80
isDone nil
Frame: 81
isDone nil
Frame: 82
isDone nil
Frame: 83
isDone nil
Frame: 84
isDone nil
Frame: 85
isDone nil
Frame: 86
isDone nil
Frame: 87
isDone nil
Frame: 88
isDone nil
Frame: 89
isDone nil
Frame: 90
isDone nil
Frame: 91
isDone nil
Frame: 92
isDone nil
Frame: 93
isDone nil
Frame: 94
isDone nil
Frame: 95
isDone nil
Frame: 96
isDone nil
Frame: 97
isDone nil
Frame: 98
isDone nil
Frame: 99
isDone nil
Frame: 100
isDone nil
Frame: 101
isDone nil
Frame: 102
isDone nil
Frame: 103
isDone nil
Frame: 104
isDone nil
Frame: 105
isDone nil
Frame: 106
isDone nil
Frame: 107
isDone nil
Frame: 108
isDone nil
Frame: 109
isDone nil
Frame: 110
isDone nil
Frame: 111
isDone nil
Frame: 112
isDone nil
Frame: 113
isDone nil
Frame: 114
isDone nil
Frame: 115
isDone nil
Frame: 116
isDone nil
Frame: 117
isDone nil
Frame: 118
isDone nil
Frame: 119
isDone nil
isDone nil
isDone nil
isDone nil
isDone nil
isDone nil
isDone nil
isDone nil

sindresorhus · 2018-02-02T07:37:59Z

The buffer looks like it's ABGR rather than BGRA.

Why is the method named gifski_add_frame_rgba and accepts RGBA8, while it's really BGRA it accepts? Sorry, my knowledge about this is very thin.

sindresorhus · 2018-02-02T08:37:27Z

I managed to implement an inefficient byte-swapping: 231408c I'm certain it can be done more efficiently, so help welcome. Probably with some kind of bit-shifting https://stackoverflow.com/a/42133405/64949

kornelski · 2018-02-02T17:01:52Z

I'll fix the isDone being nil. That's a bug.

BGRA was a typo in my github comment. It's supposed to be RGBA8.

kornelski · 2018-02-02T17:02:31Z

The inefficient byte swapping is straightforward enough that hopefully the compiler will be able to optimize it nicely.

kornelski · 2018-02-02T17:45:58Z

The progress callback with non-null arg was a bad idea, and it wouldn't work in the library implementation. I've dropped the argument (currently in git master, I'll release it as 0.8.0 later).

Now it's called only for frames. To know when the whole process is done, wait for gifski_write() to finish.

sindresorhus · 2018-02-03T07:59:05Z

BGRA was a typo in my github comment. It's supposed to be RGBA8.

Ah ok. That means the original buffer is actually ARGB.

sindresorhus · 2018-02-03T08:00:43Z

The inefficient byte swapping is straightforward enough that hopefully the compiler will be able to optimize it nicely.

Unfortunately not. Some quick testing:

debug
39 sec with byte swapping
14 sec without byte swapping

release
25 sec with byte swapping
9 sec without byte swapping

But we can worry about that later. I just want to get something out soon.

kornelski · 2018-02-03T19:34:25Z

OK, I can add swapping in Rust then (edit: done)

sindresorhus · 2018-02-05T05:26:57Z

I tried the latest master now and seems the byte-swapping is just as slow with Rust. I got 25 sec on release build now too. Can you reproduce that?

kornelski · 2018-02-05T06:43:27Z

Are you sure it's byte swapping? It doesn't show up in profiling.

1 second per frame is normal for gifski. Almost all of the time is spent on quantization and remapping, and that can't be parallelized well, because each frame depends on all previous frames.

sindresorhus · 2018-02-05T08:21:53Z

Using gifski_add_frame_rgba (The incorrect method) it only takes 10 seconds. I'm not sure if that's because of byte-swapping or because it has to do less work because the order is incorrect. Was just an observation that it got much slower after using byte-swapping.

sindresorhus · 2018-02-05T08:23:20Z

Any way to use OpenMP with the API?

kornelski · 2018-02-05T15:22:56Z

gifski_add_frame_rgba adds frames to a queue, and waits only if the queue is full. It doesn't perform any work itself, so it's not a reliable measure of time to encode. gifski_write() is.

kornelski · 2018-02-05T15:29:06Z

Xcode uses clang, and clang doesn't support OpenMP, so it's tricky.

Install gcc brew install gcc
In gifski.xcodeproj Cargo target > Info: change build $(CARGO_FLAGS) to build $(CARGO_FLAGS) --features=openmp
In theory: gifski.xcodeproj Cargo target > Build Settings: add user-defined setting CC equal to gcc-7. In practice Xcode is so buggy that this panel doesn't work, so you need to edit gifski.xcodeproj/project.pbxproj manually and add CC = "gcc-7"; You probably need to add OPENMP_STATIC=1 as well, otherwise the executable may require libraries from homebrew.

kornelski · 2018-02-05T15:32:51Z

There's also fast flag in GifskiSettings. It should double the speed at cost of ~20% of quality.

kornelski · 2018-02-05T15:33:56Z

Oh, hold off with the fast flag. I just found a bug in it when OpenMP is used :)

kornelski · 2018-02-05T15:46:37Z

OK, the fast flag is fixed. Clean build directory in Xcode to get the fix.

sindresorhus · 2018-02-17T18:21:09Z

@kornelski I'm done with the initial version of the app. (FYI, I forced pushed to clean up the commit history) There are still many things missing, like the ability to cancel the conversion, but I'd like to get it out ASAP and we can continue from there. Could you try it out and give me some feedback?

kornelski · 2018-02-17T19:35:11Z

Videos with odd widths are decoded as having an even width, which leads to stride vs width confusion:

kornelski · 2018-02-17T19:37:38Z

I am worried people will throw long-form videos in there and create GB-large gifs. Maybe hardcode some limits? e.g.

while number of frames * image size > arbitrary limit { keep halving image size }
encode max 1000 frames? these limits could be a checkbox in the save window, or removed in a later version if it gets trimming or at least abort function.

(GIF uses about 2-4bits per pixel, so hugeness of the output is predictable in advance)

sindresorhus · 2018-02-17T20:01:31Z

Videos with odd widths are decoded as having an even width, which leads to stride vs width confusion:

I thought videos could not have odd dimensions. Do you know if the problem is at the app-level here or in libgifski? I'm not really sure how to handle this.

while number of frames * image size > arbitrary limit { keep halving image size }

I don't fully understand this. You want to reduce the image size automatically? Wouldn't that be unexpected for the user. I think I better solution would be to have a image size option (in percentage of original?) in the export window.

or at least abort function.

I can easily implement the abort function.

(GIF uses about 2-4bits per pixel, so hugeness of the output is predictable in advance)

Does that mean we could estimate the resulting GIF size in the export dialog? That would be super useful

sindresorhus · 2018-02-17T20:02:24Z

So instead of trying to invent a heuristic for this, I think it's better to just present the estimated size to the user and give them the ability to downscale the GIF, set quality, and eventually trim it. What do you think?

kornelski · 2018-02-17T22:35:29Z

Yeah, that's doable.

The file size estimate can be as simple as bytes = width*height*number of frames/3.

I could add actual number of bytes written as an extra arg in the progress callback.

kornelski · 2018-02-17T22:39:56Z

Videos can have odd sizes, the same way JPEG can, despite being limited to multiple of 8 internally. The frames are encoded at size rounded up. It's most likely a quirk of the video decoder - it leaves unused pixels at end of each row. it probably reports bytes per row or stride somewhere as a separate dimension from frame width.

sindresorhus · 2018-02-18T07:23:44Z

it probably reports bytes per row or stride somewhere as a separate dimension from frame width.

CGImage has a bytesPerRow property, which we could maybe use, but I have no idea how to use it. Could gifski_add_frame_argb() accept a bytesPerRow argument and handle it in libgifski?

kornelski · 2018-02-19T00:53:53Z

Yeah, I can add it in Rust easily. I'll update the lib tomorrow.

kornelski · 2018-02-20T01:07:39Z

Fixed

kornelski · 2018-02-20T01:10:31Z

This icon is for grabs: https://gif.ski/icon.png

GIFSKI.sketch.zip

sindresorhus · 2018-02-20T04:55:59Z

From the gifski docs:

1-100, but useful range is 50-100. Recommended to set to 100.

Should I set the minimum to 50 then?

sindresorhus · 2018-02-20T05:18:05Z

The file size estimate can be as simple as bytes = widthheightnumber of frames/3.

The quality seems to affect the resulting size too. Any suggestion how I should take that into account for the estimation?

sindresorhus · 2018-02-20T05:25:46Z

Here's my progress so far with the save dialog. Feedback welcome.

sindresorhus · 2018-02-20T05:46:38Z

Regarding the icon. I like the rainbow lines, but the icon shouldn't have any text, and I think the computer thing is a little bit overdone.

How about something simple like this?

kornelski · 2018-02-20T17:40:29Z

Your icon is ok. The computer thing was supposed to be a VHS-era TV (since GIF is from 1989). The rainbow is inspired by VHS boxes: https://www.agostinoscafidi.com/wp-content/uploads/2016/07/VHS-box-series-by-Agostino-Scafidi-1024x707.jpg

kornelski · 2018-02-20T17:41:54Z

I'd guess quality 50% lowers file size by 20%.

 file size = base file size * (quality + 1.5) / 2.5

kornelski · 2018-02-20T17:44:55Z

Yes, set qualty to 50% minimum (0 on the slider could be q=50)

sindresorhus · 2018-02-22T03:38:13Z

I'd guess quality 50% lowers file size by 20%.

file size = base file size * (quality + 1.5) / 2.5

Yes. Seems to be pretty accurate.

sindresorhus · 2018-02-22T03:41:09Z

I've pushed another commit now (b3e328c) with the finished initial app. I plan to publish it to the Mac App Store when I have your 👍. Can you try it out and let me know?

I will make this repo public when it's out in the Mac App Store and we can announce it then.

sindresorhus · 2018-02-22T06:53:26Z

I'm having some problems building the app for distribution. When I build for running it works fine, but when using "Product" => "Archive" in Xcode, I end up with this error message:

SetOwnerAndGroup sindresorhus:staff /Users/sindresorhus/Library/Developer/Xcode/DerivedData/Gifski-fqctospusthmqjhbdupnamoaczvv/Build/Intermediates.noindex/ArchiveIntermediates/Gifski/InstallationBuildProductsLocation/usr/local/lib/libgifski.dylib
    cd /Users/sindresorhus/dev/private/gifski-app/gifski-api
    /usr/sbin/chown -RH sindresorhus:staff /Users/sindresorhus/Library/Developer/Xcode/DerivedData/Gifski-fqctospusthmqjhbdupnamoaczvv/Build/Intermediates.noindex/ArchiveIntermediates/Gifski/InstallationBuildProductsLocation/usr/local/lib/libgifski.dylib

chown: /Users/sindresorhus/Library/Developer/Xcode/DerivedData/Gifski-fqctospusthmqjhbdupnamoaczvv/Build/Intermediates.noindex/ArchiveIntermediates/Gifski/InstallationBuildProductsLocation/usr/local/lib/libgifski.dylib: No such file or directory
Command /usr/sbin/chown failed with exit code 1

Any ideas?

kornelski · 2018-02-22T14:26:27Z

You have to link with the static library (libgifski.a), not dylib.

kornelski · 2018-02-22T14:45:38Z

Scaling & filesize estimate help a lot! It's nice.

Can you add me to the About panel?

sindresorhus · 2018-02-22T15:19:43Z

Can you add me to the About panel?

Of course, done: 8ba2b77 The about dialog is now just a RTF file. So feel free to add a mention of the gifski library there too ;)

kornelski · 2018-02-22T15:21:35Z

Thanks!

The app is good to go!

sindresorhus · 2018-02-23T02:49:39Z

The repo is now public as we're linking to it in the app and it's a requirement that it exists for the review to be accepted.

sindresorhus changed the title ~~Problems~~ Discussion Feb 22, 2018

sindresorhus closed this as completed Feb 23, 2018

kornelski mentioned this issue Mar 1, 2018

Any way or math formula to get an estimated filesize please ? ImageOptim/gifski#28

Closed

chrismessina mentioned this issue Oct 23, 2019

Gifski told me to file a bug: "The video file is not supported." #149

Closed

sindresorhus mentioned this issue Sep 22, 2021

The trim leading video frames feature fails sometimes #254

Open

Discussion #1

Discussion #1

Comments

sindresorhus commented Feb 1, 2018 • edited Loading

kornelski commented Feb 1, 2018

sindresorhus commented Feb 1, 2018

kornelski commented Feb 1, 2018 • edited Loading

kornelski commented Feb 1, 2018

sindresorhus commented Feb 2, 2018

sindresorhus commented Feb 2, 2018

sindresorhus commented Feb 2, 2018 • edited Loading

sindresorhus commented Feb 2, 2018

kornelski commented Feb 2, 2018

kornelski commented Feb 2, 2018

kornelski commented Feb 2, 2018 • edited Loading

sindresorhus commented Feb 3, 2018

sindresorhus commented Feb 3, 2018

kornelski commented Feb 3, 2018 • edited Loading

sindresorhus commented Feb 5, 2018

kornelski commented Feb 5, 2018

sindresorhus commented Feb 5, 2018

sindresorhus commented Feb 5, 2018

kornelski commented Feb 5, 2018

kornelski commented Feb 5, 2018 • edited Loading

kornelski commented Feb 5, 2018

kornelski commented Feb 5, 2018 • edited Loading

kornelski commented Feb 5, 2018 • edited Loading

sindresorhus commented Feb 17, 2018

kornelski commented Feb 17, 2018

kornelski commented Feb 17, 2018 • edited Loading

sindresorhus commented Feb 17, 2018

sindresorhus commented Feb 17, 2018

kornelski commented Feb 17, 2018 • edited Loading

kornelski commented Feb 17, 2018

sindresorhus commented Feb 18, 2018

kornelski commented Feb 19, 2018 • edited Loading

kornelski commented Feb 20, 2018

kornelski commented Feb 20, 2018

sindresorhus commented Feb 20, 2018

sindresorhus commented Feb 20, 2018

sindresorhus commented Feb 20, 2018

sindresorhus commented Feb 20, 2018 • edited Loading

kornelski commented Feb 20, 2018 • edited Loading

kornelski commented Feb 20, 2018 • edited Loading

kornelski commented Feb 20, 2018

sindresorhus commented Feb 22, 2018

sindresorhus commented Feb 22, 2018 • edited Loading

sindresorhus commented Feb 22, 2018

kornelski commented Feb 22, 2018

kornelski commented Feb 22, 2018 • edited Loading

sindresorhus commented Feb 22, 2018

kornelski commented Feb 22, 2018

sindresorhus commented Feb 23, 2018

sindresorhus commented Feb 1, 2018 •

edited

Loading

kornelski commented Feb 1, 2018 •

edited

Loading

sindresorhus commented Feb 2, 2018 •

edited

Loading

kornelski commented Feb 2, 2018 •

edited

Loading

kornelski commented Feb 3, 2018 •

edited

Loading

kornelski commented Feb 5, 2018 •

edited

Loading

kornelski commented Feb 5, 2018 •

edited

Loading

kornelski commented Feb 5, 2018 •

edited

Loading

kornelski commented Feb 17, 2018 •

edited

Loading

kornelski commented Feb 17, 2018 •

edited

Loading

kornelski commented Feb 19, 2018 •

edited

Loading

sindresorhus commented Feb 20, 2018 •

edited

Loading

kornelski commented Feb 20, 2018 •

edited

Loading

kornelski commented Feb 20, 2018 •

edited

Loading

sindresorhus commented Feb 22, 2018 •

edited

Loading

kornelski commented Feb 22, 2018 •

edited

Loading