|
|
Created:
5 years, 9 months ago by Olle Liljenzin Modified:
5 years, 9 months ago Reviewers:
Ken Russell (switch to Gerrit), chrishtr, Peter Kasting, aelias_OOO_until_Jul13, Noel Gordon CC:
blink-reviews Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Target Ref:
refs/heads/master Project:
blink Visibility:
Public. |
DescriptionUse same parameters for jpeg decoding quality on all platforms.
This patch will change android to use same decoding parameters as
other platforms, thus changing back to dct=int and dither=fs since
the faster settings caused visible artifacts that did not justify
the small performance gain.
BUG=385515
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=192409
Patch Set 1 #
Total comments: 2
Patch Set 2 : Adding back WTF_USE_LOW_QUALITY_IMAGE_INTERPOLATION that was removed by mistake. #
Created: 5 years, 9 months ago
Messages
Total messages: 21 (4 generated)
ollel@opera.com changed reviewers: + aelias@chromium.org, kbr@chromium.org, noel@chromium.org, pkasting@chromium.org
Created a new review that replaces: https://codereview.chromium.org/339143002/
aelias@chromium.org changed reviewers: + chrishtr@chromium.org
lgtm, adding chrishtr@ for OWNERS.
owners LGTM
LGTM
lgtm
https://codereview.chromium.org/1023193002/diff/1/Source/config.h File Source/config.h (left): https://codereview.chromium.org/1023193002/diff/1/Source/config.h#oldcode89 Source/config.h:89: #define WTF_USE_LOW_QUALITY_IMAGE_INTERPOLATION 1 Seems unrelated to JPEG. Can I assume this interpolation quality flag is not used anywhere?
lgtm > https://codereview.chromium.org/1023193002/diff/1/Source/config.h > File Source/config.h (left): > > https://codereview.chromium.org/1023193002/diff/1/Source/config.h#oldcode89 > Source/config.h:89: #define WTF_USE_LOW_QUALITY_IMAGE_INTERPOLATION 1 > Seems unrelated to JPEG. Can I assume this interpolation quality flag is not > used anywhere? Seems so according to cs.chromium.org
Ran trys, not sure you have trybot access?
On 2015/03/23 05:43:00, noel gordon wrote: > Ran trys, not sure you have trybot access? I have trybot access now, but thanks for running them anyway! In the bug you wrote "While we wait... can we update the current patch". So are we still waiting for something else or should I just push the commit button now?
https://codereview.chromium.org/1023193002/diff/1/Source/config.h File Source/config.h (left): https://codereview.chromium.org/1023193002/diff/1/Source/config.h#oldcode89 Source/config.h:89: #define WTF_USE_LOW_QUALITY_IMAGE_INTERPOLATION 1 On 2015/03/21 05:49:26, noel gordon wrote: > Seems unrelated to JPEG. Can I assume this interpolation quality flag is not > used anywhere? Looks like I removed one line too much by mistake. The definition is used in Source/platform/graphics/GraphicsTypes.h.
On 2015/03/23 08:18:21, Olle Liljenzin wrote: > In the bug you wrote "While we wait... can we update the current patch". So are > we still waiting for something else or should I just push the commit button now? Folks pinged are in different times zones and busy, so let's give them time.
LGTM > On 2015/03/21 05:49:26, noel gordon wrote: > > Seems unrelated to JPEG. Can I assume this interpolation quality flag is not > > used anywhere? > > Looks like I removed one line too much by mistake. The definition is used in > Source/platform/graphics/GraphicsTypes.h. Ok good. Thanks for checking.
Alexis mentioned to me off-line, that our YUV JPEG decode path should be ok, as long as jpeg_read_raw_data() does not depend on the IDCT method, which he was pretty sure it doesn't. https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... Maybe have a scan around in that library function olle@ and let me know.
On 2015/03/23 13:52:47, noel gordon wrote: > Alexis mentioned to me off-line, that our YUV JPEG decode path should be ok, as > long as jpeg_read_raw_data() does not depend on the IDCT method, which he was > pretty sure it doesn't. > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > > Maybe have a scan around in that library function olle@ and let me know. I tried to follow the code which isn't that easy. To my limited understanding this function is just decompressing data, not decoding it.
Let's land it then.
The CQ bit was checked by noel@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org, chrishtr@chromium.org, pkasting@chromium.org, aelias@chromium.org Link to the patchset: https://codereview.chromium.org/1023193002/#ps20001 (title: "Adding back WTF_USE_LOW_QUALITY_IMAGE_INTERPOLATION that was removed by mistake.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1023193002/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://src.chromium.org/viewvc/blink?view=rev&revision=192409 |