-
Notifications
You must be signed in to change notification settings - Fork 18k
image/gif: generated image cannot be opened in xv and crashes OmniWeb 3.x web browser #33748
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
XV debug mode says this:
|
It's possibly the same issue as #13746 (comment) with the eog ("Eye of GNOME") image viewer. It's hard to say definitively, though. It's fair enough to say that Go's GIF encoder is writing something that old image viewers can't read, but it's hard to do something about that (in the Go code) without knowing what it is specifically those old image viewers don't like. And figuring out what they don't like probably means spelunking in xv or OmniWeb code, not in Go code. Or, as I said in issue #13746, if it's:
then it's really up to EoG to fix their bug. Which they might already have done (in GNOME/gdk-pixbuf@b88f1ce, not sure if it's rolled out to stable distro's yet). |
I also think that this may be to do with LZW compression. Would it be possible to add an option to disable LZW compression when creating GIFs? |
I don't think you can have a GIF image without LZW compression. This is in terms of the GIF file format, not any one specific encoder or decoder. |
I think I may have determined the source of this problem, in part at least. The generated GIFs have their color resolution bits and global color table size zeroed out. This is part of the Logical Screen Descriptor's packed byte. (See https://www.matthewflickinger.com/lab/whatsinagif/bits_and_bytes.asp for more information.) (Edit: while it is a problem that this data is not being written properly, it's apparently not the main issue for the library I'm having trouble with. It may be compression clear codes, still investigating.) The problem can be fixed using GraphicsMagick: Related issues: nothings/stb#1222 skyjake/lagrange#353 makew0rld/didder#7 |
Looking here it would seem that this information should be written properly: https://cs.opensource.google/go/go/+/refs/tags/go1.16.8:src/image/gif/writer.go;l=158;drc=refs%2Ftags%2Fgo1.16.8 Unless, that is, the gif data doesn't include a ColorModel/Palette. But it appears that this is populated correctly during Encode(), as far as I can tell. |
The author of the stb library, which also has trouble parsing these images, says the problem may be with the lack of a "clear code" which should be present per the GIF RFC: nothings/stb#1222 (comment) |
To help with debugging this, I generated a 1x1 gif. Here's the hex of it before and after the GraphicsMagick "fix": Broken:
Fixed:
|
OK, so with some digging into the actual bytes I am certain that the issue has something to do with the format of the image data bytes (last section before the trailer). Original file:
Fixed file:
Disregarding the application extension block (GraphicsMagick must have felt this was necessary despite the lack of animation), it's obvious that there is a difference in the image data. I manually edited the hex data of the broken image to match the data of the fixed image, and lo and behold, the image is now working. Here's the hex of the manually corrected image:
The new image is one byte longer. |
OK, checking the LZW bytes, it appears that the author of the stb library was correct. The code is not outputting a clear code before it begins writing the LZW data. Every source I come across treats this as mandatory, except for the W3C spec which says it "should" be included. (In this case, though, I think that "should" was actually intended to be "MUST," especially in light of how many older implementations handle it as a requirement.) Perhaps the blockWriter setup() function could write a clear code (here: https://cs.opensource.google/go/go/+/refs/tags/go1.16.8:src/image/gif/writer.go;l=73;drc=refs%2Ftags%2Fgo1.16.8)? The clear code is 2 * the LZW code size (litWidth as defined here: https://cs.opensource.google/go/go/+/refs/tags/go1.16.8:src/image/gif/writer.go;l=327;drc=refs%2Ftags%2Fgo1.16.8;bpv=0;bpt=1). Edit: ok, it's not quite as simple as just writing a clear code since the data length also changes, but writing that code should fix the issue for libraries that expect the data to start with a conventional clear code. |
Fixing this issue will probably also close #26108 |
Change https://golang.org/cl/354710 mentions this issue: |
@mntn-xyz thanks for the excellent and thorough debugging. I have sent out a fix for review. |
Thank you for fixing this!!! |
Great to see this fixed, thank you! I assume the fix will find its way into the next release, Go 1.17.3? |
I expect it to release in Go 1.18. If you must have it sooner, you can always fork the |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
yes, any release
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
Create a simple gif image:
What did you expect to see?
Expected to open in older image viewers, editors and browsers.
What did you see instead?
A number of old browsers, image viewers and editors report generated gif image as corrupted or display it mangled. For example xv image viewer, https://en.wikipedia.org/wiki/Xv_(software) says the file is turncated. OmniWeb browser (v2.x and v3.x) crashes. Most graphical software on SGI IRIX reports it corrupted.
The text was updated successfully, but these errors were encountered: