-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
De-zigzag JPEG's DQT when loading; deprecate convert_dict_qtables #4989
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
Changes from 2 commits
9980981
4dc1953
a1d8d63
70c7514
d11c794
afb6ad2
dfeb49c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -252,7 +252,7 @@ def DQT(self, marker): | |
| data = array.array("B" if precision == 1 else "H", s[1:qt_length]) | ||
| if sys.byteorder == "little" and precision > 1: | ||
| data.byteswap() # the values are always big-endian | ||
| self.quantization[v & 15] = data | ||
| self.quantization[v & 15] = [data[i] for i in zigzag_index] | ||
| s = s[qt_length:] | ||
|
|
||
|
|
||
|
|
@@ -585,9 +585,11 @@ def _getmp(self): | |
|
|
||
|
|
||
| def convert_dict_qtables(qtables): | ||
| qtables = [qtables[key] for key in range(len(qtables)) if key in qtables] | ||
| for idx, table in enumerate(qtables): | ||
| qtables[idx] = [table[i] for i in zigzag_index] | ||
| warnings.warn( | ||
| "convert_dict_qtables is deprecated and will be removed in a future" | ||
| " release. Conversion is no longer needed.", | ||
radarhere marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| DeprecationWarning, | ||
| ) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A thought (feel free to disagree) - if we're considering this to be a public method that requires deprecation, then should we maybe just leave the functionality there (still adding in the deprecation warning)?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I searched GitHub for this function: https://github.com/search?q=convert_dict_qtables+language%3Apython+-filename%3AJpegPresets.py. There are not many uses. Only the first two pages contain interesting results. The rest is just copies of Pillow itself. They all take the If Pillow de-zigzags the tables when loading (as the rest of this PR does), Most people don't need to mess with quantization tables. But there's obviously more code than just what I found on GitHub and I don't know how representative these uses are. I guess there could also be cases where people are getting qtables from somewhere other than straight from an image's I guess Pillow could temporarily add a new
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @radarhere What do you think? Shall we merge this PR as is? (And can explicitly replace "a future version" with "Pillow 10 (2023-01-02)")
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure - I can see now why |
||
| return qtables | ||
|
|
||
|
|
||
|
|
@@ -668,7 +670,9 @@ def validate_qtables(qtables): | |
| qtables = [lines[s : s + 64] for s in range(0, len(lines), 64)] | ||
| if isinstance(qtables, (tuple, list, dict)): | ||
| if isinstance(qtables, dict): | ||
| qtables = convert_dict_qtables(qtables) | ||
| qtables = [ | ||
| qtables[key] for key in range(len(qtables)) if key in qtables | ||
| ] | ||
| elif isinstance(qtables, tuple): | ||
| qtables = list(qtables) | ||
| if not (0 < len(qtables) < 5): | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.