-Duse_system_libjpeg=1 unsupported and broken?

[Paweł Hajdan, Jr.]

Chromium bug http://code.google.com/p/chromium/issues/detail?id=106954 has been closed WontFix, and Gentoo bug https://bugs.gentoo.org/show_bug.cgi?id=393471 has even more info about the issue.

Why is -Duse_system_libjpeg=1 unsupported and broken? It worked fine for a very long time, and in fact Linux distros which recompile Chromium are not affected by http://code.google.com/p/chromium/issues/detail?id=31427, because all packages are compiled against the same version of libjpeg. That issue only affects Google Chrome, which is always built with -Duse_system_libjpeg=0.

There is a more detailed analysis of the incompatibility here: https://bugs.gentoo.org/show_bug.cgi?id=393471#c12 .

In my opinion it's a bit worrying that now we require a specific variant of libjpeg (i.e. libjpeg-turbo), at a specific, very recent, version. I think it's fine to take advantage of more advanced features if they're available, but being able to work with unmodified system libraries helps to ensure we've not diverged too much.

What do you think? Is there a chance to make -Duse_system_libjpeg=1 work again?

[Hironori Bono (坊野 博典)]

Greetings,

Apologies for this compatibility issue.
WebKit r101286 <http://trac.webkit.org/changeset/101286> used an
option of libjpeg-turbo to avoid unnecessary copies in decoding JPEG
files. (Even though it improves its performance by ~30%, it has to
depend on libjpeg-turbo 1.1.90+ as written in
<http://crbug.com/106954>.) Is it acceptable for you to use this
option only when we use our copy of libjpeg-turbo?

Regards,

Hironori Bono
E-mail: hb...@chromium.org

> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev

[Paweł Hajdan, Jr.]

2011/12/12 Hironori Bono (坊野 博典) <hb...@chromium.org>

Is it acceptable for you to use this
option only when we use our copy of libjpeg-turbo?

Sounds good, use this option only when -Duse_system_libjpeg=0. Thank you for taking a look!

Should http://code.google.com/p/chromium/issues/detail?id=106954 be re-opened then? The main problem for Linux distros is not really the version number of libjpeg-turbo (it's easy to add a dependency on high enough version), but rather the incompatibility with vanilla libjpeg as noted in https://bugs.gentoo.org/show_bug.cgi?id=393471#c4 .

[Hironori Bono (坊野 博典)]

Greetings Pawel,

Many thanks for noticing this issue.
I have re-opened the bug <http://crbug.com/106954> and filed WebKit
Bug 74286 <http://webkit.org/b/74286>.

Regards,

Hironori Bono
E-mail: hb...@chromium.org

[Paweł Hajdan, Jr.]

ping ping - what's the status of those bugs?

The developer of libjpeg-turbo has posted some comments on https://bugs.gentoo.org/show_bug.cgi?id=393471 - they can be very helpful for you if you want to do runtime detection of the extensions. You can also comment on that bug and he should also receive your comments.

In case you're not able to fix the problem soon, could you please revert the breaking patch?

Note that the problematic change is in the non-Chromium-specific code, so I'm guessing that other commonly used in Open Source ports like Qt and GTK haven't noticed it yet only because they move slower than Chromium.

[Brett Wilson]

On Mon, Jan 2, 2012 at 10:24 AM, Paweł Hajdan, Jr.
<phajd...@chromium.org> wrote:
> ping ping - what's the status of those bugs?
>
> The developer of libjpeg-turbo has posted some comments
> on https://bugs.gentoo.org/show_bug.cgi?id=393471 - they can be very helpful
> for you if you want to do runtime detection of the extensions. You can also
> comment on that bug and he should also receive your comments.
>
> In case you're not able to fix the problem soon, could you please revert the
> breaking patch?

I don't understand whether the change that broke this is worthwhile or
was just a test or what. But I don't think that we should be reverting
code that we think is correct because some people want to compile
Chromium using not-technically-supported modes like
use_system_libjpeg. If people want that mode to work, those should fix
it.

Brett

[Evan Martin]

Another option is to rename the variable to use_system_libjpeg_turbo
to make the dependency more clear.

[Mike Frysinger]

On Tue, Jan 3, 2012 at 17:11, Evan Martin wrote:

and then rename it again when libjpeg_turbo gets replaced by another
rice-flavor-of-the-month jpeg implementation ?
-mike

[Nico Weber]

I think Evan's point was that it could be clearer that we're not using
stock libjpeg. use_system_libjpeg_rice_flavor would work just as well.

Nico

[Mike Frysinger]

On Tue, Jan 3, 2012 at 17:29, Nico Weber wrote:

right, and my counter point was that mentioning it in the standard
distro/builder facing documentation should be sufficient (wherever
that is)

seems like build/install-build-deps.sh still says libjpeg rather than
the turbo variant

to the original point, the jpeg variants are supposed to be drop-in
replacements and sticking to the API/ABI exported by the canonical
libjpeg implementation. mainline webkit shouldn't be going off into
the weeds ...
-mike

[Evan Martin]

On Tue, Jan 3, 2012 at 2:34 PM, Mike Frysinger <vap...@chromium.org> wrote:
>> I think Evan's point was that it could be clearer that we're not using
>> stock libjpeg. use_system_libjpeg_rice_flavor would work just as well.
>
> right, and my counter point was that mentioning it in the standard
> distro/builder facing documentation should be sufficient (wherever
> that is)
>
> seems like build/install-build-deps.sh still says libjpeg rather than
> the turbo variant

Most of the use_system_foobar aren't used by Google Chrome, so the
code and doc paths are only exercised by downstream users like the
Gentoo ones who started this thread. The docs frequently bitrot, and
it falls on people who care (maybe you?) to try to fight that.

[Mike Frysinger]

/me looks at Pawel who actually knows how to build this behemoth ;)
-mike

[Evan Martin]

On Tue, Jan 3, 2012 at 3:06 PM, Mike Frysinger <vap...@chromium.org> wrote:
>>> seems like build/install-build-deps.sh still says libjpeg rather than
>>> the turbo variant
>>
>> Most of the use_system_foobar aren't used by Google Chrome, so the
>> code and doc paths are only exercised by downstream users like the
>> Gentoo ones who started this thread.  The docs frequently bitrot, and
>> it falls on people who care (maybe you?) to try to fight that.
>
> /me looks at Pawel who actually knows how to build this behemoth ;)

Ah, I'd guessed by your email address that you had commit access. Sorry!

[Mike Frysinger]

nah, i'm one of those "CrOS" lackeys. when it comes to the browser,
i'm just a backseat driver throwing peanuts.
-mike

[Paweł Hajdan, Jr.]

On Tue, Jan 3, 2012 at 23:48, Evan Martin <ev...@chromium.org> wrote:

Most of the use_system_foobar aren't used by Google Chrome

I don't think Google Chrome uses any of the use_system_foo switches. If it wants to use a system lib, it just does it. Switches are to override Chrome defaults, and they usually work without trouble. Most of the time problems that show up are missing gyp dependencies, which are also Chrome bugs. I take care of those issues.

 

code and doc paths are only exercised by downstream users like the

Gentoo ones who started this thread.

... and Debian, Ubuntu PPA, Arch Linux, BSDs, all the other distros that ship Chromium. Don't forget about other WebKit ports that are likely to hit this incompatibility too.

Anyway, I could easily change the dependency on Gentoo to just require a recent enough libjpeg-turbo. But that locks the Chromium project (and, consequently, Google Chrome), to one particular flavor of libjpeg, and with the ABI broken on the Chromium side.

Note that the libjpeg-turbo developer is willing to help here, and he's already made changes on the libjpeg-turbo side. If you want to keep hacks in WebKit that break ABI compatibility, that's your call. I can change deps to anything on distro side (not that it's a good idea, but it's easy).

However, a proper fix would be to do the recognition of the JPEG extensions at run time, not compile time (read DRC's comments at https://bugs.gentoo.org/show_bug.cgi?id=393471). And distros testing the code in various different ways you don't (like compiling against libjpeg-turbo and using it with vanilla libjpeg at run time) can actually help increasing the quality of the interface between Chrome/WebKit and libjpeg-turbo. Currently it relies on some hacks, but it doesn't have to.

I'd like to emphasize an important point:

On Tue, Jan 3, 2012 at 23:34, Mike Frysinger <vap...@chromium.org> wrote:

the jpeg variants are supposed to be drop-in

replacements and sticking to the API/ABI exported by the canonical
libjpeg implementation.  mainline webkit shouldn't be going off into
the weeds ...

. 

[Noel Gordon]

On Jan 3, 5:55 am, Brett Wilson <bre...@chromium.org> wrote:

> I don't understand whether the change that broke this is worthwhile or
> was just a test or what.

The change increases JPEG decoding performance by an additional ~2x.

> But I don't think that we should be reverting
> code that we think is correct because some people want to compile
> Chromium using not-technically-supported modes like
> use_system_libjpeg. If people want that mode to work, those should fix
> it.

Indeed. use_system_libjpeg=0 is the only mode we test/support since
moving
to libjpeg-turbo for desktop chrome. Our libjpeg-turbo library is
alpha-ish
software, we need to security patch it, tune it for speed, and enable
web
features such as MJPG support.

~noel

[Noel Gordon]

On Jan 4, 7:29 pm, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:
> On Tue, Jan 3, 2012 at 23:48, Evan Martin <ev...@chromium.org> wrote:
> > Most of the use_system_foobar aren't used by Google Chrome
>
> I don't think Google Chrome uses any of the use_system_foo switches. If it
> wants to use a system lib, it just does it. Switches are to override Chrome
> defaults, and they usually work without trouble.

And what about the webkit Android port, or ChromeOS? ChromeOS uses
libjpeg6b via use_system_libjpeg=1 for example.

> > code and doc paths are only exercised by downstream users like the
> > Gentoo ones who started this thread.
>
> ... and Debian, Ubuntu PPA, Arch Linux, BSDs, all the other distros that
> ship Chromium. Don't forget about other WebKit ports that are likely to hit
> this incompatibility too.

WebKit ports run the webkit tests and those tests easily detect
libjpeg-turbo
bugs https://bug-75189-attachments.webkit.org/attachment.cgi?id=120491

> Anyway, I could easily change the dependency on Gentoo to just require a
> recent enough libjpeg-turbo. But that locks the Chromium project (and,
> consequently, Google Chrome), to one particular flavor of libjpeg, and with
> the ABI broken on the Chromium side.

libjpeg-turbo's flavor is ABI libjpeg6b down to the decoded byte.
libjpeg6b is
the gold standard for jpeg decoding/encoding. The ABI is not broken.

> Note that the libjpeg-turbo developer is willing to help here, and he's
> already made changes on the libjpeg-turbo side. If you want to keep hacks
> in WebKit that break ABI compatibility, that's your call. I can change deps
> to anything on distro side (not that it's a good idea, but it's easy).
>
> However, a proper fix would be to do the recognition of the JPEG extensions

> at run time, not compile time (read DRC's comments athttps://bugs.gentoo.org/show_bug.cgi?id=393471).

> And distros testing the code in various different ways you don't (like compiling against
> libjpeg-turbo and using it with vanilla libjpeg at run time) can actually
> help increasing the quality of the interface between Chrome/WebKit and
> libjpeg-turbo. Currently it relies on some hacks, but it doesn't have to.

Our libjpeg-turbo is current. Chrome security continuously fuzzes it
so it is
secure. We tune it for speed so chrome has the fastest jpeg of any
browser
by a good margin. It ain't no "hack". We don't need to test various
libjpeg,
and have no desire to run chrome against them at run time.

> I'd like to emphasize an important point:
>
> On Tue, Jan 3, 2012 at 23:34, Mike Frysinger <vap...@chromium.org> wrote:

> > *the jpeg variants are supposed to be drop-in*
> > *replacements and sticking to the API/ABI exported by the canonical

> > libjpeg implementation.  mainline webkit shouldn't be going off into

> > the weeds ...*

Webkit is not off in the weeds. The canonical implementation is
libjpeg6b
and that's what libjpeg-turbo provides. Maybe refer to Tom Lane here:

http://lists.fedoraproject.org/pipermail/devel/2010-May/136976.html

You have a patch to remove my change from webkit at Gentoo and that'll
allow you to compile chromium against a questionable libjpeg-turbo, or
the even more questionable libjpeg7 or libjpeg8. You will compromise
the speed, maybe even the security, of chrome if you go that way. I'm
not sure that's a great idea. I suggest you use our libjpeg-turbo and
stop fiddling with our use_system_libjpeg build flag.

~noel

[Paweł Hajdan, Jr.]

On Tue, Jan 17, 2012 at 04:10, Noel Gordon <no...@chromium.org> wrote:

libjpeg-turbo's flavor is ABI libjpeg6b down to the decoded byte.

libjpeg6b is the gold standard for jpeg decoding/encoding.  The ABI is not broken.

Sorry about the confusion, I meant that when the browser is compiled against libjpeg-turbo and then run against libjpeg the images are not displayed, as reported in https://bugs.gentoo.org/show_bug.cgi?id=393471#c3

Also, Noel explained to me that in fact libjpeg-turbo and libjpeg are not guaranteed to be binary compatible (again, in terms of executable/library linking, not the image on-disk format), which is explained in http://libjpeg-turbo.svn.sourceforge.net/viewvc/libjpeg-turbo/trunk/libjpeg.txt?revision=299&view=markup, near "as a shared library".

 

It ain't no "hack".

Sorry for calling it a hack. With above taken into account it is not, and I was really confused about actual compatibility promises of libjpeg.

 

We don't need to test various libjpeg, and have no desire to run chrome against them at run time.

Now I strongly object to any statements like "we" above. I'm a committer, and it's very important to me to make the project a good Open Source citizen. Using system libraries is how software compiled from source is packaged by Linux distros. Note that this does not affect Google Chrome at all, where I have no objections about using bundled libjpeg-turbo.

Anyone who enables -Duse_system_libjpeg=1 is obviously responsible for the result (so that there is no need for Chromium developers to do the test), and I'd like that switch to remain available.

Finally, I'm now exploring possible options on the Gentoo side, and started the following thread: http://archives.gentoo.org/gentoo-dev/msg_a1abdea87d28c72e19daef15bca9c949.xml . I'll wait for the results from that discussion and then post an update here.