Added orientation, compression and id_section as TGA save keyword arguments #3327
Conversation
radarhere
changed the title
src/PIL/TgaImagePlugin.py
Outdated
| if orientation > 0: | ||
| flags = flags | 0x20 | ||
|
|
||
| fp.write(b"\000" + | ||
| fp.write(o8(idlen) + |
There was a problem hiding this comment.
There is a small problem: o8 is bytes((i & 255,)), which will not raise an exception like struct.pack(), making it possible for fp.write() below to write id_section bigger than the written idlen, resulting in a broken image.
There was a problem hiding this comment.
Okay, I've added a commit to trim id_section.
| @@ -151,11 +151,19 @@ def _save(im, fp, filename): | |||
| except KeyError: | |||
| raise IOError("cannot write mode %s as TGA" % im.mode) | |||
|
|
|||
| rle = im.encoderinfo.get("rle", False) | |||
|
|
|||
| if "rle" in im.encoderinfo: | |||
There was a problem hiding this comment.
I think it's safe to remove the "rle" keyword in favor of "compression".
There was a problem hiding this comment.
Yeah. I'm just not keen to break the backwards compatibility of this one. If this PR is merged, you're welcome to submit a separate PR for that.
There was a problem hiding this comment.
Sounds sensible to keep it, we've no idea what code might be using im.encoderinfo["rle"].
We'd need deprecation warnings first before removing.
There was a problem hiding this comment.
TGA-specifc flags are not currently documented, so it's unlikely that anyone uses them.
src/PIL/TgaImagePlugin.py
Outdated
| im.info.get("id_section", "")) | ||
| idlen = len(id_section) | ||
| if idlen > 255: | ||
| idlen = 255 |
There was a problem hiding this comment.
Okay, I've added a commit. Note that the commit also changes the idlen variable in _open, for consistency.
| @@ -151,11 +151,19 @@ def _save(im, fp, filename): | |||
| except KeyError: | |||
| raise IOError("cannot write mode %s as TGA" % im.mode) | |||
|
|
|||
| rle = im.encoderinfo.get("rle", False) | |||
|
|
|||
| if "rle" in im.encoderinfo: | |||
There was a problem hiding this comment.
Sounds sensible to keep it, we've no idea what code might be using im.encoderinfo["rle"].
We'd need deprecation warnings first before removing.
Resolves #3226