Bug 98878 – Fix JPEG turbo decoding failure when IMAGE_DECODER_DOWN_SAMPLING is enabled

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Jaehun Lim]

When using libjpeg-turbo and enabling IMAGE_DECODER_DOWN_SAMPLING,
JPEG decoding failed because of no support for JCS_EXT_RGBA, JCS_EXT_BGRA.
Set RGBA values when color space is JCS_EXT_RGBA or JCS_EXT_BGRA.

[Jaehun Lim]

Created attachment 167962 [details]
Patch

[WebKit Review Bot]

Attachment 167962 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jaehun Lim]

Created attachment 167963 [details]
Patch

[Kenneth Russell]

This looks fine, but I'd like Noel to do an unofficial review before r+'ing it.

[noel gordon]

Looks sane.  Sadly no tests exist for the IMAGE_DECODER_DOWN_SAMPLING enable.

[Kenneth Russell]

Comment on attachment 167963 [details]
Patch

That's unfortunate, but I assume having this work is better than not. r=me

[WebKit Review Bot]

Comment on attachment 167963 [details]
Patch

Clearing flags on attachment: 167963

Committed r131075: <http://trac.webkit.org/changeset/131075>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[noel gordon]

Someone asked me a question about this.  @jaehun - should the #if test be as follows?

   #if ENABLE(IMAGE_DECODER_DOWN_SAMPLING) && defined(TURBO_JPEG_RGB_SWIZZLE)

Alternatively, we could remove all this and ensure that ports that use IMAGE_DECODER_DOWN_SAMPLING can use libjoeg-turbo correctly.   TURBO_JPEG_RGB_SWIZZLE should not be taken to mean libjpeg-turbo and I think we should fix that.  Thoughts?

[Jaehun Lim]

(In reply to comment #9)
> Someone asked me a question about this.  @jaehun - should the #if test be as follows?
> 
>    #if ENABLE(IMAGE_DECODER_DOWN_SAMPLING) && defined(TURBO_JPEG_RGB_SWIZZLE)
>
I considered the same #if statement at first. I have no objection to change #if statement.

> Alternatively, we could remove all this and ensure that ports that use IMAGE_DECODER_DOWN_SAMPLING can use libjoeg-turbo correctly.   TURBO_JPEG_RGB_SWIZZLE should not be taken to mean libjpeg-turbo and I think we should fix that.  Thoughts?

I perfectly agree with you.

[Yong Li]

Just noticed this fix. Nice job. For those images are not scaled when DOWNSAMPLE is enabled, it seems that we can still use the swizzling path?

[Yong Li]

(In reply to comment #11)
> Just noticed this fix. Nice job. For those images are not scaled when DOWNSAMPLE is enabled, it seems that we can still use the swizzling path?

Bug 103749 created for this, and patch is uploaded

[noel gordon]

I have reviewed that patch, looks good.

Second thing to do is to handle the scaled case better per comment 10.