Bug 50054 - [Chromium] Use libjpeg-turbo instead of libjpeg
Summary: [Chromium] Use libjpeg-turbo instead of libjpeg
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-24 23:10 PST by Hironori Bono
Modified: 2010-12-14 11:32 PST (History)
6 users (show)

See Also:


Attachments
A refactoring change (3.52 KB, patch)
2010-12-07 00:58 PST, Hironori Bono
no flags Details | Formatted Diff | Diff
The second change (4.15 KB, patch)
2010-12-07 20:43 PST, Hironori Bono
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hironori Bono 2010-11-24 23:10:50 PST
(Copied from <http://crbug.com/48789> since we need to change the 'WebKit.gyp' file.)

> libjpeg-turbo is binary compatible with libjpeg but 2-4x faster.
>
> Worth looking into to make Chrome even faster?
Comment 1 Hironori Bono 2010-12-07 00:58:04 PST
Created attachment 75789 [details]
A refactoring change
Comment 2 WebKit Review Bot 2010-12-07 09:06:29 PST
Attachment 75789 [details] did not pass style-queue:

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 WebKit Review Bot 2010-12-07 10:07:14 PST
Attachment 75789 [details] did not pass style-queue:

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 WebKit Review Bot 2010-12-07 11:08:23 PST
Attachment 75789 [details] did not pass style-queue:

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 WebKit Review Bot 2010-12-07 12:09:32 PST
Attachment 75789 [details] did not pass style-queue:

Failed to run "[u'git', u'reset', u'--hard', u'refs/remotes/trunk']" exit_code: 128
error: Could not write new index file.
fatal: Could not reset index file to revision 'refs/remotes/trunk'.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 WebKit Review Bot 2010-12-07 15:45:52 PST
Attachment 75789 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/6820096
Comment 7 Hironori Bono 2010-12-07 20:43:33 PST
Created attachment 75866 [details]
The second change

Oops, my previous change causes a build break on canary bots because of the lack of 'libjpeg_gyp_path'. I have updated this change so it works well with WebKit canary bots.

Regards,

Hironori Bono
Comment 8 WebKit Review Bot 2010-12-07 21:38:34 PST
Attachment 75789 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2
Updating OpenSource
Incomplete data: Delta source ended unexpectedly at /usr/lib/git-core/git-svn line 5061

Died at WebKitTools/Scripts/update-webkit line 132.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 WebKit Review Bot 2010-12-07 21:59:40 PST
Attachment 75866 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2
Updating OpenSource
Incomplete data: Delta source ended unexpectedly at /usr/lib/git-core/git-svn line 5061

Died at WebKitTools/Scripts/update-webkit line 132.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 WebKit Review Bot 2010-12-10 09:17:11 PST
Comment on attachment 75866 [details]
The second change

Clearing flags on attachment: 75866

Committed r73736: <http://trac.webkit.org/changeset/73736>
Comment 11 WebKit Review Bot 2010-12-10 09:17:17 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Tony Chang 2010-12-10 10:15:27 PST
Comment on attachment 75866 [details]
The second change

View in context: https://bugs.webkit.org/attachment.cgi?id=75866&action=review

> WebCore/ChangeLog:12
> +        This change replaces the hard-coded path to the GYP files of the JPEG
> +        library with a 'libjpeg_gyp_path' variable, which is added by Chromium
> +        r68453. (When building WebKit Chromium, this change sets its value to
> +        '<(chromium_src_dir)/third_party/libjpeg/libjpeg.gyp' to avoid changing
> +        the current behavior.)

Just so I understand, this makes use use regular libjpeg for WebKit only checkouts and libjpeg-turbo when in a chromium checkout, right?  Can we have WebKit only checkouts use libjpeg-turbo too?
Comment 13 Hironori Bono 2010-12-12 19:28:33 PST
Greetings Tony,

Thank you for your comments.

(In reply to comment #12)
> Just so I understand, this makes use use regular libjpeg for WebKit only checkouts and libjpeg-turbo when in a chromium checkout, right?

Yes, you are totally correct. :)

> Can we have WebKit only checkouts use libjpeg-turbo too?

Unfortunately, not for now. We notice we need to update our patched yasm to build libjpeg-turbo on Mac because of yasm Ticket #186. <http://www.tortall.net/projects/yasm/ticket/186>. (Our yasm is based on yasm 0.8, which does not integrate the fix for this bug.) When we update our yasm and verified we can build libjpeg-turbo on Mac, I will send another change to switch our WebKit build to use libjpeg-turbo. (It changes "WebKit/chromium/DEPS" and pull libjpeg-turbo and our patched yasm to the WebKit tree.) By the way, we can use libjpeg-turbo only on Windows and on Mac since Linux uses the system JPEG library.
Is it OK for you?

Regards,

Hironori Bono
Comment 14 Tony Chang 2010-12-14 11:32:22 PST
(In reply to comment #13)
> Unfortunately, not for now. We notice we need to update our patched yasm to build libjpeg-turbo on Mac because of yasm Ticket #186. <http://www.tortall.net/projects/yasm/ticket/186>. (Our yasm is based on yasm 0.8, which does not integrate the fix for this bug.) When we update our yasm and verified we can build libjpeg-turbo on Mac, I will send another change to switch our WebKit build to use libjpeg-turbo. (It changes "WebKit/chromium/DEPS" and pull libjpeg-turbo and our patched yasm to the WebKit tree.) By the way, we can use libjpeg-turbo only on Windows and on Mac since Linux uses the system JPEG library.
> Is it OK for you?

Sounds good to me, thanks Hironori!