Hook up use_system_libjpeg for Android

Hook up use_system_libjpeg for Android

Also enable it by default when builing with the Android build system.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=147850

[Hironori Bono, 2012-07-19 00:37:15]

Greetings,

If I recall correctly, Android currently uses libjpeg-turbo to use NEON. Is it
OK to use system libjpeg by default?

Regards,

Hironori Bono

[Noel Gordon, 2012-07-19 06:26:05]

https://bugs.webkit.org/show_bug.cgi?id=75861#c6 onwards.

[Noel Gordon, 2012-07-19 06:30:22]

Also the idea that libjpeg-turbo is a drop-in replacement for libjpeg breaks
down if the advanced features of libjpeg-turbo are used as we found out the hard
way on https://wkb.ug/59670 once the changes rolled in linux distros.

[Steve Block, 2012-07-19 15:47:27]

Thanks for the replies.

Yes, Android currently uses libjpeg-turbo, and we'll continue to do so for the
NDK/SDK build. This change modified the default only when building in the
Android tree.

Thanks for the pointers Noel, we'll be sure to keep an eye on those tests once
we have Chromium running in the Android tree.

[Hironori Bono, 2012-07-20 02:10:58]

Greetings,

Thanks for your explanation. In my honest opinion, I'm not an expert of Android
and like to hear thoughts of some Android engineers who worked for integrating
libjpeg-turbo to Android. (As Noel noted, some WebKit code depends on
libjpeg-turbo and I would like to verify this switch does not cause
regressions.)

Regards,

Hironori Bono

[Steve Block, 2012-07-20 10:20:57]

> I'm not an expert of Android
> and like to hear thoughts of some Android engineers who worked for integrating
> libjpeg-turbo to Android.
Sure, added Sami

> (As Noel noted, some WebKit code depends on
> libjpeg-turbo and I would like to verify this switch does not cause
> regressions.)
If I understand the bugs correctly, WebKit shouldn't require libjpeg_turbo. The
bugs were cases where the libjpeg_turbo-optimised path made invalid assumptions,
which have now been fixed, no?

[Sami, 2012-07-20 10:57:20]

On 2012/07/20 10:20:57, Steve Block wrote:
> > (As Noel noted, some WebKit code depends on
> > libjpeg-turbo and I would like to verify this switch does not cause
> > regressions.)
> If I understand the bugs correctly, WebKit shouldn't require libjpeg_turbo.
The
> bugs were cases where the libjpeg_turbo-optimised path made invalid
assumptions,
> which have now been fixed, no?

That's right -- WebKit should work equally well with both libjpeg and
libjpeg-turbo. We did hit some bugs in libjpeg-turbo itself earlier, but those
have been resolved. So, this switch should not cause any issues on Android.

[Hironori Bono, 2012-07-20 11:29:31]

Greetings,

Even though WebKit should not require libjpeg-turbo, WebKit uses some features
of libjpeg-turbo when it is compiled with it. (Noel landed WebKit changes that
used a libjpeg-turbo feature to improve decoding speed by >20%.)
In my honest opinion, I do not have any backgrounds about this change and wonder
why this change likes to use the system libjpeg when building with the Android
build and our copy of libjpeg-turbo when building with NDK. (As you can imagine,
supporting two libraries increases our maintenance cost.)

Regards,

Hironori Bono

[Steve Block, 2012-07-20 13:03:40]

Thanks for the reply Hironori.

We're working on using Chromium to power the Android system WebView API. For
this work, we build Chromium using the Android build system.

The motivation for this change is to reduce the total ROM size by making use of
the libjpeg library already provided by the Android system, rather than
statically linking Chromium's copy of libjpeg_turbo.

You're right that this will introduce a performance penalty. Indeed, it may turn
out that this penalty is too large to justify the ROM saving. However, we'll be
in a better position to make that decision if we can test with both
alternatives.

Regarding maintenance cost, the cost to Android-specific code or gyp files is
very small. Or do you mean the general cost of Chromium supporting the existing
use_system_libjpeg flag? Are there plans to remove this?

[Hironori Bono, 2012-07-21 06:06:59]

Greetings Steve,

LGTM. Many thanks for your explanation. This explanation is convincing for me.
By the way, this 'use_system_libjpeg' flag caused several build problems on
external developers who created Chromium packages for Linux distributions. (In
my personal opinion, I wonder if this option is worth supporting as a person who
fixed them.)

Regards,

Hironori Bono

http://codereview.chromium.org/10807019/diff/1/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/10807019/diff/1/build/common.gypi#newcode517
build/common.gypi:517: # Enable to use system libjpeg.
nit: is it possible to add a comment to describe the reason why Android use the
system libjpeg, e.g. "Use system libjpeg to reduce the ROM footprint"?

[Steve Block, 2012-07-23 12:41:09]

http://codereview.chromium.org/10807019/diff/1/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/10807019/diff/1/build/common.gypi#newcode517
build/common.gypi:517: # Enable to use system libjpeg.
On 2012/07/21 06:07:00, Hironori Bono wrote:
> nit: is it possible to add a comment to describe the reason why Android use
the
> system libjpeg, e.g. "Use system libjpeg to reduce the ROM footprint"?

Done.

[commit-bot: I haz the power, 2012-07-23 13:35:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/steveblock@chromium.org/10807019/9001