-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Added orientation, compression and id_section as TGA save keyword arguments #3327
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's safe to remove the "rle" keyword in favor of "compression".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's snake_case
this variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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