[needs-packaging] libjpeg-turbo

Reported by Yves Glodt on 2010-08-01
208
This bug affects 37 people
Affects Status Importance Assigned to Milestone
Mozilla Firefox
Fix Released
Medium
Debian
Fix Released
Unknown
Ubuntu
Wishlist
Unassigned
Nominated for Maverick by Peyotll

Bug Description

libjpeg-turbo seems to greatly improve speed compared to libjpeg.

Read more here:
http://libjpeg-turbo.virtualgl.org/About/Performance
http://fedoraproject.org/wiki/Features/libjpeg-turbo

Maybe it would be good to package it, or at least offer it in a ppa.

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.4) Gecko/20100611 Firefox/3.6.4
Build Identifier:

To quote the URL
"libjpeg-turbo is a high-speed version of libjpeg for x86 and x86-64 processors which uses SIMD instructions (MMX, SSE2, etc.) to accelerate baseline JPEG compression and decompression. libjpeg-turbo is generally 2-4x as fast as the unmodified version of libjpeg, all else being equal.

libjpeg-turbo was originally based on libjpeg/SIMD by Miyasaka Masaru, but the TigerVNC and VirtualGL projects made numerous enhancements to the codec, including improved support for Mac OS X, 64-bit support, support for 32-bit and big endian pixel formats (RGBA, ABGR, etc.), accelerated Huffman encoding/decoding, and various bug fixes. The goal was to produce a fully open source codec that could replace the partially closed source TurboJPEG/IPP codec used by VirtualGL and TurboVNC. libjpeg-turbo generally achieves 80-120% of the performance of TurboJPEG/IPP. It is faster in some areas but slower in others."

Fedora already working on incorporating it in their distribution.
http://fedoraproject.org/wiki/Features/libjpeg-turbo

License is wxWindows license for some parts and LGPL for the rest.

This would close a lot of bugs regarding libjpeg/SSE Optimization/64-bit Porting

Reproducible: Always

You'll probably need to consult with <email address hidden> about the license. See guidelines here: http://www.mozilla.org/MPL/license-policy.html

When looking at third party libraries for the video decoding and YCbCr conversion LGPL was an issue.

Using libjpeg-simd is probably an easier option here than libjpeg-turbo license wise.

According to
http://libjpeg-turbo.svn.sourceforge.net/viewvc/libjpeg-turbo/trunk/README-turbo.txt

"Some of the optimizations to the Huffman encoder/decoder were borrowed from
VirtualGL, and thus the libjpeg-turbo distribution, as a whole, falls under the
wxWindows Library Licence, Version 3.1. A copy of this license can be found in
this directory under LICENSE.txt. The wxWindows Library License is based on
the LGPL but includes provisions which allow the Library to be statically
linked into proprietary libraries and applications without requiring the
resulting binaries to be distributed under the terms of the LGPL.

The rest of the source code, apart from these modifications, falls under a less
restrictive, BSD-style license (see README.)"

So it seems that wxWindows license is a more liberal version of LGPL and the rest is BSD (I was mistaken in the initial posting)

(In reply to comment #2)
> Using libjpeg-simd is probably an easier option here than libjpeg-turbo license
> wise.

Sorry for the bug spam

Regarding libjpeg-simd

From the Google translation of http://cetus.sakura.ne.jp/softlab/jpeg-x86simd/jpegsimd.html

"In addition, AMD64 (EM64T/Intel64) in 64bit mode environment (currently) not supported."

"AMD64 compatible version is under construction...(October 26, 2006: The production process is currently stopped. Seems slow release schedule. Sorry.)"

Jpeg-simd would have to be ported to 64-bit which from what I have read is a cumbersome task.

(In reply to comment #4)
> Jpeg-simd would have to be ported to 64-bit which from what I have read is a
> cumbersome task.

From the diffs between the 64-bit version and the 32-bit version it seems that the process would be mostly mechanical. In fact, it should be possible to use the same source for 64-bit as 32-bit like libvpx does. Further, the 64-bit versions in libjpeg-turbo should be under the same license as jpeg-simd so we should be able to just use them directly.

Hi, all. I'm the maintainer and one of the creators of libjpeg-turbo, so I thought I'd throw in some comments. For starters, libjpeg-turbo is not licensed under pure LGPL. It uses the wxWindows Library License, which eliminates some of the viral provisions of the LGPL. My understanding of the differences between the two is spelled out here:

http://www.virtualgl.org/About/License

I have confirmed with the wxWindows developers who created the license that this matches their understanding as well. Essentially, it is our understanding that wxWindows nullifies Sections 6-7 of LGPL v2.1, which means you can statically link the Library with any application without requiring the application as a whole to fall under the provisions of the LGPL. You are still bound by other sections of the LGPL, which means you would still be required to distribute source for the Library along with the application.

That being said, however, I can understand the desire for a fully unencumbered version of the source. The only part of the code which is licensed under wxWindows is the Huffman encoding/decoding optimizations (portions of jchuff.c and jdhuff.c.) These were developed while I was a Sun employee, so unfortunately I don't have the right to dual license them, or else I certainly would do so.

What I could do, however, is develop a mechanism whereby you could easily build the code without the Huffman optimizations ('configure --without-huffman-opt' or something like that.) This would simply involve building libjpeg-turbo with the "stock" versions of jchuff.c and jdhuff.c from libjpeg-6b. I could even go one step farther and make it so that the wxWindows-licensed files are not included in the distribution tarball if you 'configure --without-huffman-opt' and then 'make dist'.

This would slow down the overall performance by 20-25%, but it will still be miles faster than regular libjpeg on x86/x86-64 systems.

What DRC suggests seems like the smart thing to do, to begin with. Let's take the parts which are unambiguously BSD and still provide a significant win.

We currently have a policy of only taking code which is compatible with all three of our licenses. We'd have to do some analysis on the wxWindows licence to see whether it fits our policy. (Please, people, stop rolling your own licences! :-) If someone could open another bug for that, that would be great.

But if it's not compatible in that way, I think it's highly unlikely we'd want to introduce a new licence into the Mozilla mix just for a 25% win in JPEG decoding (particularly if we'd already recently had a big win over what we'd been using for years).

Gerv

wxWindows is actually a very mature license at this point-- it just isn't well known. It was created to solve a specific problem -- allowing static linking of an open source library with proprietary code without forcing the proprietary application to fall under the terms of the LGPL, but also guaranteeing the open nature of the code (which BSD-style licenses do not do.) OpenSceneGraph uses it, and that's where I learned about it originally.

Let me know if there's anything I can do to facilitate this on my end. If you're just going to merge the code into your build tree, then you can just as easily replace jchuff.c and jdhuff.c with their original versions when you do that, but I'm still willing to add a configure switch to libjpeg-turbo as well.

(In reply to comment #8)
> Let me know if there's anything I can do to facilitate this on my end. If
> you're just going to merge the code into your build tree, then you can just as
> easily replace jchuff.c and jdhuff.c with their original versions when you do
> that, but I'm still willing to add a configure switch to libjpeg-turbo as well.

This seems like it might be best option.

Has any work been done on ARM or other platforms?

(In reply to comment #10)
> Has any work been done on ARM or other platforms?

One thing I've been meaning to do is switch mobile to use the IFAST dct instead of the ISLOW dct. Android uses IFAST and in fact have an ARM implementation of the IFAST dct. Unfortunately, it is licensed with the apache license. Perhaps we can convince Google to relicense it?

(In reply to comment #10)
> Has any work been done on ARM or other platforms?

We should write NEON SIMD code if we change to libjpeg-turbo.

(In reply to comment #11)
> (In reply to comment #10)
> > Has any work been done on ARM or other platforms?
>
> One thing I've been meaning to do is switch mobile to use the IFAST dct instead
> of the ISLOW dct. Android uses IFAST and in fact have an ARM implementation of
> the IFAST dct. Unfortunately, it is licensed with the apache license. Perhaps
> we can convince Google to relicense it?

Past year, Michael Moy tried to change to IFAST on some platforms by bug 416157. But many test cases became failed. If changing to IFAST, we have to modify many test cases, too.

DRC: the problem is not just incompatibility, but simplicity. A library under the straight LGPL would be compatible with the MPL, the LGPL and the GPL (our three licences) but would increase the licensing complexity of our codebase, something we have historically tried to avoid.

Gerv

So, just so I understand, inbound code to the Mozilla project has to either be licensed under the tri-license or something less restrictive that can be re-licensed under the tri-license? So if it were possible to re-license the offending code in libjpeg-turbo under your tri-license, then that would solve the problem?

Our goal is that people should be able to redistribute all the code that goes into our products under the terms of any one of the MPL, LGPL, or GPL. So:
-- code that is licensed with a license compatible with all of those (e.g., BSD) is fine
-- code that is licensed with multiple license options that cover the three above licenses is also fine (e.g., dual licensed MPL/LGPL, since the LGPL option is compatible with the GPL as well)

Note that we don't relicense other people's code. E.g., when we import BSD-licensed code into our tree, it keeps the original license.

Unless I'm missing something fundamental, I think wxWindows-licensed code could be legally redistributed under all three licenses. However, I fully understand the desire not to introduce new licenses into your code tree.

One question-- do you currently copy the libjpeg code into the Mozilla source or do you build it separately?

(In reply to comment #16)
> One question-- do you currently copy the libjpeg code into the Mozilla source
> or do you build it separately?

It's in our tree, but it's not by any means the original libjpeg, as it's been patched to pieces.

http://mxr.mozilla.org/mozilla-central/source/jpeg/

OK, then it seems as if merging in the "turbo" features (minus the Huffman coding) into your existing code tree would be the way to go.

(In reply to comment #13)
> DRC: the problem is not just incompatibility, but simplicity. A library under
> the straight LGPL would be compatible with the MPL, the LGPL and the GPL (our
> three licences) but would increase the licensing complexity of our codebase,
> something we have historically tried to avoid.

Original license of SIMD code is http://cetus.sakura.ne.jp/softlab/jpeg-x86simd/jpegsimd.html#conditions

In Japanese, "この SIMD 拡張版 IJG JPEG software の使用条件については、オリジナル版の IJG JPEG software の使用条件が適用されます。"

In English by Google Translate, "This SIMD extension for IJG JPEG software terms of use, the original version of the IJG JPEG software is subject to terms of use."

So, original author who is MIYASAKA-san says that this code is same license as libjpeg. I believe that license is no problem if we port MIYASAKA-san's code to our tree, not using libjpeg-turbo.

Gerv, how about?

Why does the idea of using the old libjpeg/SIMD code keep coming up in this discussion? This has already been covered multiple times-- libjpeg/SIMD does not have 64-bit support or many of the other bug fixes we made to libjpeg-turbo.

libjpeg-turbo also bears the same license as libjpeg, if you don't use our accelerated version of jchuff.c and jdhuff.c.

In short-- merge in libjpeg-turbo, minus jchuff.c and jdhuff.c, and there is no licensing problem.

There seems to be some confusion around what should happen here... the right thing, I think, is:

1. Look at our in-tree libjpeg and how it differs from stock libjpeg.

2. Based on that, either merge the delta from libjpeg -> libjpeg-turbo into our tree, OR merge delta of stock libjpeg and in-tree libjpeg into libjpeg-turbo, and then pull that in.

(minus j[cd]huff.c, of course).

I don't really know how extensive the changes in #1 are, and I don't know what, if any, conflicts there would be between the changes for -turbo and our tree version of libjpeg. But I think it's worth it to find out; the perf wins wouldn't hurt as we try to do decode on draw, and it sounds like there are some pretty significant ones here.

Forgot to ask -- who wants to make a patch? :-)

After a cursory look at the code, it seems like libjpeg-turbo may want to borrow Mozilla's enhancements, where applicable. In particular, the use of static tables in choice areas like RGB->YUV conversion is likely to improve our performance as well.

Also, there is some of the code which is going to conflict, because your version already has some SIMD acceleration. I think that most of it could be replaced with ours, but the areas in which you use SSE2 would probably need to be benchmarked against our version to see which is faster (integer IDCT, for instance.)

(In reply to comment #23)
> After a cursory look at the code, it seems like libjpeg-turbo may want to
> borrow Mozilla's enhancements, where applicable. In particular, the use of
> static tables in choice areas like RGB->YUV conversion is likely to improve our
> performance as well.
>
> Also, there is some of the code which is going to conflict, because your
> version already has some SIMD acceleration. I think that most of it could be
> replaced with ours, but the areas in which you use SSE2 would probably need to
> be benchmarked against our version to see which is faster (integer IDCT, for
> instance.)

Our integer SSE2 idct is buggy (bug 477728), so while it would be nice to see how the performance compares, I think I would still switch to the libjpeg-turbo one even if ours is faster.

(In reply to comment #22)
> Forgot to ask -- who wants to make a patch? :-)

I'll try.

(In reply to comment #6)
> What I could do, however, is develop a mechanism whereby you could easily build
> the code without the Huffman optimizations ('configure --without-huffman-opt'
> or something like that.)

My guess is that we'll not want to include j{c,d}huff.c in our source tree at all, if they're encumbered with an incompatible license. Is this right?

tags: added: needs-packaging
summary: - Evalutate and package libjpeg-turbo
+ [needs-packaging] libjpeg-turbo
38 comments hidden view all 208 comments
Brian Murray (brian-murray) wrote :

*** This is an automated message ***

This bug is tagged needs-packaging which identifies it as a request for a new package in Ubuntu. As a part of the managing needs-packaging bug reports specification, https://wiki.ubuntu.com/QATeam/Specs/NeedsPackagingBugs, all needs-packaging bug reports have Wishlist importance. Subsequently, I'm setting this bug's status to Wishlist.

Changed in ubuntu:
importance: Undecided → Wishlist
Götz Christ (g-christ) wrote :

Fedora 14 will use libjpeg-turbo by default.

From http://fedoraproject.org/wiki/Features/libjpeg-turbo
"libjpeg-turbo is at least twice faster in JPEG compression/decompression than original libjpeg on platforms with MMX/SSE instruction set. It has same API/ABI like original libjpeg and also runs on non-SSE platforms where is around 25% faster."

"libjpeg-turbo has same API/ABI as libjpeg so no package needs to be rebuilt "

This should be considered for Maverick.

spaetz (spaetz) wrote :

libjpeg-turbo is a drop-in replacement for libjpeg, so +1

ghepeu (ghepeu) wrote :

I second the request.

I'm usually very skeptic about purported performance improvements, but I did my tests on a couple of machines and I think that given the ubiquity of JPEG pictures the upgrade to libjpeg-turbo could be one of the most important user-visible speed improvements in the last few releases of Ubuntu.

I compiled libjpeg-turbo 1.0.1 and selected with LD_LIBRARY_PATH the new libjpeg.so.62.0.0 instead of the system library provided by Ubuntu 10.04.

On a Athlon 4850e (dual core 64bit cpu, 32 bit userspace) working at 2.5 Ghz, converting a not-so-large RPG PPM image (7200x4500) to and from JPEG (average of 5 runs):

time convert image.ppm image.jpg

libjpeg-classic real 2.709s user 2.364s sys 0.318s
libjpeg-turbo real 1.646s user 1.301s sys 0.317s

time convert image.jpg image.ppm

libjpeg-classic real 2.130s user 1.703s sys 0.359s
libjpeg-turbo real 1.292s user 0.872s sys 0.356s

On a Acer Aspire One 532h netbook with a Atom N450 cpu (64 bit userspace) working at 1.67 Ghz, the same test gives:

time convert image.ppm image.jpg

libjpeg-classic real 5.763s user 5.996s sys 0.770s
libjpeg-turbo real 2.612s user 2.900s sys 0.720s

time convert image.jpg image.ppm

libjpeg-classic real 4.588s user 3.816s sys 0.858s
libjpeg-turbo real 2.602s user 1.692s sys 0.868s

Occasionally I use the netbook to stream video from the integrated webcam to another pc with this gstreamer pipeline:

gst-launch-0.10 v4l2src ! video/x-raw-yuv,width=640,height=480 ! ffmpegcolorspace ! jpegenc ! multipartmux ! udpsink host=192.168.1.2 port=5000

With the frequency forced to 1GHz the cpu usage is:

libjpeg-classic 37-38%
libjpeg-turbo 10-11%

With the frequency forced to 1.67Ghz the cpu usage is:

libjpeg-classic 22-23%
libjpeg-turbo 6-7%

More importantly, when the default ondemand governor is used, with libjpeg-turbo the frequency stays mostly at 1 Ghz, while with libjpeg-classic it divides almost equally between 1 Ghz and 1.67Ghz, with the obvious consequences in terms of battery life.

On this machine more than on the faster desktop pc the difference between libjpeg-turbo and libjpeg-classic is clearly visible: when nautilus is thumbnailing the images in a directory, when I open an image with oeg, when I'm looking at a f-spot gallery, when I change wallpaper, etc. etc. The upgrade should be considered before 10.10.

rogerdpack (rogerpack2005) wrote :

+1 for wishing this were used as the default libjpeg distro so the various programs could benefit from it (zoneminder, for instance, benefits greatly).

Don Cady (doncady) wrote :

I concur. I know this is ancillary, but libjpeg-turbo sigificantly improved/decreased my proc usage on my zoneminder install.
Also, It looks like Mozilla is working on bundling this in FF now: https://bugzilla.mozilla.org/show_bug.cgi?id=573948
See that bug for their licensing considerations, as this is apparently under the 'wxWindows Library License'.

Changed in debian:
status: Unknown → New
foobar (gagarin) wrote :

i hope it gets into debian and ubuntu soon. and i hope people don't start to do what mozilla wants to do (bundle more software into their huge monster of software). can't wait to run feh faster on my computers to view pretty large photo images...

David Robert Lewis (afrodeity) wrote :

more speed, lower profile, greater productivity please.

williamts99 (williamts99) wrote :

This might need some more testing, I installed the latest libjpeg-turbo and could not open pdf documents with document viewer. Reverted to libjpeg62 and the problem went away.

foobar (gagarin) wrote :

did you install from source or are there any debian (source) packages around?

Steve Langasek (vorlon) wrote :

If anyone is seeing the same problem as williamts99 (who is not subscribed to this bug), could you please provide a copy of /proc/cpuinfo from the affected system, and a copy of an example pdf that the problem is reproducible with?

If possible, please also try building a package from the git branch at "git://git.linaro.org/people/tomgall/meego/libjpeg-turbo.git linaroize" to see if the problem is still reproducible for you.

I'm having no problems here viewing pdfs with this package built for amd64. My suspicion is that the only problem is for users trying to use this on pre-SSE x86 systems... which is not surprising since most of the improvements over jpeg upstream here involve hand-coded assembly to take advantage of vector instructions.

Tom Gall (tom-gall) wrote :

It's also available in my ppa:tom-gall/packages.

Like Steve, I haven't been able to replicate any issues but thus far I've been testing on armel.

Changed in firefox:
importance: Unknown → Medium
status: Unknown → In Progress
155 comments hidden view all 208 comments

Oh, I see; that's a change we made, but which isn't in libjpeg-turbo.

Until it's fixed upstream, why don't we just guard the #define's in jmorecfg.h? We already keep that file out of sync with libjpeg-turbo's mainline.

(In reply to comment #155)
> Oh, I see; that's a change we made, but which isn't in libjpeg-turbo.
>
> Until it's fixed upstream, why don't we just guard the #define's in jmorecfg.h?
> We already keep that file out of sync with libjpeg-turbo's mainline.

That sounds fine.

Created attachment 522445
Patch v2.0. Part 1: Upgrade to libjpeg-turbo v1.1.0

Created attachment 522446
Patch v2.0. Part 2: Address review comments.

Created attachment 522449
Interdiff v1.3.1 -> v2.0 part 1

Created attachment 522459
Interdiff v1.3.1 -> v2.0 part 1 without deleted files

This interdiff is the same as attachment 522449, but it doesn't include the files I deleted, in an attempt to make it easier to read.

DRC, what would be a good medium for handling Jeff's remaining review comments?

I'm going to push the latest patches to try now.

Created attachment 522544
Proposed upstream patch for addressing concern about jround_up() argument types.

I'll address the review comments that are relevant to the upstream code here:

(1) jround_up() argument types: this was the result of a bad game of Celebrity Whack-a-Mole that I played while trying to port the code to Win64. Jeff's idea of creating a local, power-of-2-specific round-up function for jmemmgr.c is a better idea, and I've done so in a new patch (attachment 522544) (actually, I made it a macro instead of a function.) Please let me know if that looks like a reasonable solution.

(2) RGB_{RED|GREEN|BLUE} macro conflicts on OS/2 (OS/2? Really?): This seems like an innocuous enough patch, although I'd rather have it in jmorecfg.h instead of jpeglib.h. I'd also like to know more of the back story as to why OS/2 is defining those macros. Is this a generic problem that anyone building libjpeg-turbo on OS/2 would encounter? My lingering doubt here is that libjpeg supports OS/2, so I wonder why they haven't encountered this problem on that platform. Is Mozilla including some header file that causes those macros to be defined?

(3) bitscan instructions: I tested the proposed patch to make jcdctmgr.c use GCC 3.4+ compiler intrinsics to replace flss() with bitscan instructions, but I cannot observe any measurable speedup from this (tested several different image types and platforms, as well as 32-bit and 64-bit.) If it were even 2% faster, I would agree to include it, but a performance patch that causes no appreciable increase in performance does nothing but needlessly obfuscate the code.

(4) The tables in jccolor.c are part of the optimized RGB-to-grayscale conversion routines. These are pre-computed R-to-luminance, G-to-luminance, and B-to-luminance conversion values. The problem with this approach is that it loses a bit of accuracy due to round-off, so I recently checked in code to the SVN trunk (1.2 working version) that uses new SIMD routines to perform RGB-to-grayscale conversion. This is faster and eliminates the round-off issues, as well as the need for the tables in jccolor.c and the #ifdef in rgb_gray_convert().

(5) The rest of the concerns regarding jccolor.c have already been addressed in trunk as well (pulling rgb_red, rgb_green, rgb_blue out of the loop, etc.)

> (2) RGB_{RED|GREEN|BLUE} macro conflicts on OS/2 (OS/2? Really?): This seems
> like an innocuous enough patch, although I'd rather have it in jmorecfg.h
> instead of jpeglib.h.

Attachment 522446 puts the #undef's in jmorecfg.h, right above the #defines.

And just FYI, another thing that I'm currently being contracted to do is investigate rewriting the performance optimizations in j{c|d}huff*. This will hopefully allow me to re-license those files in libjpeg-turbo 1.2.

Comment on attachment 522544
Proposed upstream patch for addressing concern about jround_up() argument types.

>+#define PAD(size, align) (((size) + (align) - 1) & (~((align) - 1)))

I usually prefer functions over macros for the additional type safety and to avoid evaluating arguments twice (e.g. PAD(x, a++))

(In reply to comment #163)
> (2) RGB_{RED|GREEN|BLUE} macro conflicts on OS/2 (OS/2? Really?): This seems
> like an innocuous enough patch, although I'd rather have it in jmorecfg.h
> instead of jpeglib.h. I'd also like to know more of the back story as to why
> OS/2 is defining those macros. Is this a generic problem that anyone building
> libjpeg-turbo on OS/2 would encounter? My lingering doubt here is that libjpeg
> supports OS/2, so I wonder why they haven't encountered this problem on that
> platform. Is Mozilla including some header file that causes those macros to be
> defined?

I'd be fine with dropping this altogether. If OS/2 still needs it, we can always add it back.

>
> (3) bitscan instructions: I tested the proposed patch to make jcdctmgr.c use
> GCC 3.4+ compiler intrinsics to replace flss() with bitscan instructions, but I
> cannot observe any measurable speedup from this (tested several different image
> types and platforms, as well as 32-bit and 64-bit.) If it were even 2% faster,
> I would agree to include it, but a performance patch that causes no appreciable
> increase in performance does nothing but needlessly obfuscate the code.

It's probably worth adding a comment to flss saying that it's not performance critical enough to justify using compiler intrinsics.

> (4) The tables in jccolor.c are part of the optimized RGB-to-grayscale
> conversion routines. These are pre-computed R-to-luminance, G-to-luminance,
> and B-to-luminance conversion values. The problem with this approach is that
> it loses a bit of accuracy due to round-off, so I recently checked in code to
> the SVN trunk (1.2 working version) that uses new SIMD routines to perform
> RGB-to-grayscale conversion. This is faster and eliminates the round-off
> issues, as well as the need for the tables in jccolor.c and the #ifdef in
> rgb_gray_convert().

That sounds like a good solution to me.

(In reply to comment #165)
> And just FYI, another thing that I'm currently being contracted to do is
> investigate rewriting the performance optimizations in j{c|d}huff*. This will
> hopefully allow me to re-license those files in libjpeg-turbo 1.2.

This sounds great.

Created attachment 522583
New proposed upstream patch for addressing concern about jround_up() argument types.

(In reply to comment #166)
> >+#define PAD(size, align) (((size) + (align) - 1) & (~((align) - 1)))
>
> I usually prefer functions over macros for the additional type safety and to
> avoid evaluating arguments twice (e.g. PAD(x, a++))

My experience has been that the compiler does a good job of optimizing out that duplication of effort, and the lack of overhead of the additional function call usually makes up for it, but in the specific case we're referring to, it doesn't matter enough to worry about, and using a function call is more consistent with the way it worked before. Re-submitted as attachment 522583.

(In reply to comment #170)
> (In reply to comment #166)
> > >+#define PAD(size, align) (((size) + (align) - 1) & (~((align) - 1)))
> >
> > I usually prefer functions over macros for the additional type safety and to
> > avoid evaluating arguments twice (e.g. PAD(x, a++))
>
> My experience has been that the compiler does a good job of optimizing out that
> duplication of effort, and the lack of overhead of the additional function call
> usually makes up for it, but in the specific case we're referring to, it
> doesn't matter enough to worry about, and using a function call is more
> consistent with the way it worked before. Re-submitted as attachment 522583

I was referring to the fact that PAD(x, a++) does a++ twice which is not what the caller is likely to expect. Regardless, I like the new version better.

Ah, the orange is due to unexpected success. That's not so bad. :)

I think we can just back out bug 582850.

http://hg.mozilla.org/mozilla-central/rev/4643426a1523

Yep, and this should close bug 477728

Created attachment 522674
Patch v2.0 Part 3: Re-enable reftests on Windows.

Why is support for arithmetic encoding/decoding disabled?

Don't we need to use the libjpeg v8 ABI in order to get this?

(In reply to comment #176)
> Why is support for arithmetic encoding/decoding disabled?

We haven't decided whether to enabled this yet. This is a separate issue and needs a separate bug as well as a lot more thought/research.

(In reply to comment #177)
> Don't we need to use the libjpeg v8 ABI in order to get this?

No, it works when emulating the v6b ABI as well. It just adds two additional functions without breaking compatibility with the rest of the ABI.

Apparently this causes OOM in yasm 0.8.something we may need to up the min version

More specifically, with yasm 0.8.0.2194, which is the latest available by default with Ubuntu 10.04. yasm 1.1.0.2352 works fine.

Fedora 13 has old yasm too

(Though, honestly we'd really prefer to make it buildable with nasm, or gas. But that's another bug...)

Changed in firefox:
status: In Progress → Fix Released

NOTE: libjpeg-turbo trunk now contains a version of jdhuff.c and jdhuff.h (Huffman decoder) with the optimizations re-written and re-licensed under the same BSD-style license as the rest of libjpeg. Looking into jchuff* as well, but I figured you were probably more interested in the decoder than the encoder.

I've filed bug 650899 for updating libjpeg-turbo.

Jesse, why did you nominate this bug for tracking-firefox5?

Because bsmedberg told me to in http://groups.google.com/group/mozilla.dev.planning/msg/bfc2fabaecacdd97. Also because I think it should be listed on https://wiki.mozilla.org/Features/Release_Tracking to make sure it gets the right security reviews, license file additions, changelog entries, crash stats scrutiny, and whatever else people follow that page for.

NOTE: libjpeg-turbo trunk now contains a version of jchuff.c
(Huffman encoder) with the optimizations re-written and re-licensed under the
same BSD-style license as the rest of libjpeg.

... and the bug for tracking that is bug 650899.

That bug is for tracking the decoder. I'm talking about the encoder now.

FYI, the Chrome team has apparently done a security review of the library:
http://code.google.com/p/chromium/issues/detail?id=48789#c3

Security team finished fuzzing as planned, we are good here.

Oibaf (oibaf) wrote :

It looks like libjpeg-turbo is in Ubuntu since oneiric.

Changed in ubuntu:
status: New → Fix Released
Changed in debian:
status: New → Fix Committed
Changed in debian:
status: Fix Committed → Fix Released
Displaying first 40 and last 40 comments. View all 208 comments or add a comment.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Related blueprints

Remote bug watches

Bug watches keep track of this bug in other bug trackers.