Replies: 2 comments 6 replies
-
https://www.fileformat.info/format/gif/egff.htm
So I'm inclined to agree that comments shouldn't exist an on per-image basis.
I realise it may seem completely obvious to you, but could you spell out for me exactly how Appendix B says this?
https://en.wikipedia.org/wiki/GIF states that "Graphic Control Extension (comment fields precede this in most files)", so it may be better to have it before, not after.
Up until this point, I was following your argument to have all of the comments together, but now I'm lost. Are you trying to account for the fact that when loading a GIF file, it might be a file that Pillow has saved previously with repeated comments? I think it would be reasonable to just combine all of the comment blocks into a single string when reading. If the comment block is duplicated like Pillow has been doing up until now, then only one copy could be used.
What do you feel is missing? |
Beta Was this translation helpful? Give feedback.
-
I’ll work on this. |
Beta Was this translation helpful? Give feedback.
-
Pillow's handling of comments in GIF files is broken.
I hesitated to write this as an issue because I'm not sure where Pillow should go with this.
Current handling of GIF comment blocks appears to be as follows. On reading, comment blocks are merged on a per-image basis. That is, they are seen before an image and merged into the frame's
.info["comment"]
dict. If a frame image is preceded by separate comment blocks "foo" and "bar" they will appear as "foobar" inframe.info["comment"]
. I believe this is wrong. Comments that appear after the last image frame are lost completely, because there's no frame associated with them.Input comments before the first image block will be (by default) written on output when
.save()
is called. If acomment=
arg is in the.save()
it will override the input comment. Comments after the first image will be lost unless the user takes care to grab them from theframe.info
dicts.Currently only one comment can be written, and it will be duplicated in every frame. This is an error similar to the current handling of the Netscape looping block (fixed by @radarhere in PR #6211).
Also, they are in the wrong place in the output file; they are placed between the Graphic Control Block and the Image Descriptor Block. I know the spec says of the GCB "The scope of this Extension is the graphic rendering block that follows it; it is possible for other extensions to be present between this block and its target." (23d in spec) But it also has a EBNF grammar (Appendix B) of allowable block structure that does not admit anything between the GCB and the Image block. I suspect that some programs (MS Teams maybe?) may not like a comment there.
Is it OK that the canonical methods of Pillow reading and writing of GIF files do not preserve most comments? I mean code like:
I started a PR to address all this but decided to see if anyone else cares before putting more work into it.
I was thinking along these lines: Normally code as above would preserve at least all the comments except any after the last image. If a
comment=
arg is given to.save()
then it would be written just after the Netscape looping block (if any). An emptycomment=""
orcomment=None
could suppress existing comment blocks. Maybe some other convention (a new arg?) could leave only the user'scomment=
and suppress other comments. Also, maybe comments after the last image could be stored internally in a variableself.comment
and made available to the user. Maybe they could also be saved in the main.encoderinfo
to be written to output.At the least, I would like to see the
comment=
arg (or comment retained from the input) get written once, at the front of the output file. I can do a PR for that if @radarhere and/or others agree.BTW the documentation could use some work too.
Beta Was this translation helpful? Give feedback.
All reactions