Mozilla is using a SIMD-accelerated JPEG decorder in Fx5. We should try to adapt this to VMX using what we will have learned from WebM. Medium as we're still reasonably fast at this even in C.

https://bugzilla.mozilla.org/show_bug.cgi?id=573948

Dropping milestone since we're not going to do this yet.

Would be far to complex to implement all the SIMD code IMO.

But I noticed that Mac OS X (10.4 and 10.5) have a heavily Altivec accelerated version of libjpeg inside the ImageIO framework which is a sub framework of ApplicationServices framework. Disassembling shows clearly all the altivec acceleration code. I tried to link to that library instead of the one in the mozilla sources. All exported symbols are prefixed with cg so I had to create a modified header. It links without problems but TFF unfortunately crashes upon decoding JPEG files. Apples seems to have changed something with the buffers...

Alternatively one might adapt nsJPEGDecoder to use the ImageIO framework.

Would be far to complex to implement all the SIMD code IMO.

But I noticed that Mac OS X (10.4 and 10.5) have a heavily Altivec accelerated version of libjpeg inside the ImageIO framework which is a sub framework of ApplicationServices framework. Disassembling shows clearly all the altivec acceleration code. I tried to link to that library instead of the one in the mozilla sources. All exported symbols are prefixed with cg so I had to create a modified header. It links without problems but TFF unfortunately crashes upon decoding JPEG files. Apples seems to have changed something with the buffers...

Alternatively one might adapt nsJPEGDecoder to use the ImageIO framework.

That code should help us in order to use the ImageIO framework for our purposes.

http://vlists.pepperfish.net/pipermail/netsurf-dev-netsurf-browser.org/2011-March/002398.html

It was implemented only months ago as the JPEG importer for the Mac OS X port of the netsurf browser (interesting project!).

Some success in directly using the libJPEG.dylib from ImageIO.framework: the djpeg command line decompressor works with the altivec accelerated libJPEG (verified by setting a breakpoint in the altivec accelerated IDCT function). So I'll continue with that path...

The only worry I have about that is that if there's a bug or security flaw in Apple's libJPEG, it's not going to get fixed. I think using this is worthwhile in the near term, but we need to have a fallback (I presume back to the Mozilla in-tree libjpeg with or without AltiVec) if a security issue crops up.

Well, I'm actually using the configure flag "--with-system-libjpeg" (don't know if written correctly), so I only have to supply a modified header adding the "cg" prefix to the libjpeg function calls and a link to the library.

For now I simply added both the headers and the library to the Xcode SDK and reconfigured with "--with-system-libjpeg". So that's pretty clean.

More success! Decoding JPEGs actually works as long as all the data is actually loaded completely upon decoding. So I can view images I load from the hard disk and also from the web as long as I set the inital paint delay high enough! I also think I already discovered the issue; libjpeg decompressing is called although there isn't any new data available. This seems to happen because an internal counter indicates the contrary. libjpeg should in fact recognize that situation, I think. I'll introduce further controls before the function calls to the library.

It does finally work, with an extra check that the image stream source address passed to libjpeg isn't NULL!

The attached patch works provided you're linking against the Mac OS X 10.4 SDK which must be located at "/Developer/SDKs/MacOS10.4u.sdk". The symlink that's included in the patch might not be created the right way - if not you'll have to manually construct it (in the file "jpeg-imageio/lib/libjpeg.dylib" you see the path to link to).

Furthermore you have to add "ac_add_options --with-system-jpeg=$topsrcdir/jpeg-imageio" to your mozconfig in order to activate it.

That extra check for the source address not being NULL to me seems to be a good idea anyway and even in case the mozilla libjpeg (or any other one) is used it saves the function call that would return some cycles later with the information that there wasn't any data available.

I did a speed test by dragging and dropping a large jpeg file into a TFF7 window. By simple counting the seconds it needed to appear on my PowerBook Wallstreet G4/500 there was an improvement of roughly 20%.

TODO: - the verification might be done in an inline function in the "jpeglib.h" header, also doing the wrapping - the jpeg compressor isn't tested yet (where is it used?)

Damn, that's pretty nice work. I'm still nervous about this for 7, especially since 7 final is pretty close, but let's get this into 8 along with the other proposed optimizations.

One thing. If I read your patch right, we should be able to use this on all architectures; we let the SDK determine if AltiVec gets enabled, yes? We can just add this and let OS X sort it out for G3 vs G4/G5.

Yes, the Apple jpeg lib should determine the CPU type and choose altivec acceleration according to it. I didn't test it but I have a G3 CPU to put in my PowerBook Wallstreet to test it with - but I would have to rebuild the entire sources because I bult with auto-vectorization enabled.

As for the release version: Why don't make an intermediate version based on 7? That would have the benefit of having less parts changed at once than releasing it with 8.

There might be a 7.0.1 if I don't get the 8 widget changes rewritten in time, and it would appear in that if we have to go that route.

I'm trying to eliminate the symlink, however: that doesn't fit nicely with our change sets or hg. If we have to have it, it's probably better to have the build system generate it and do so in an unmonitored directory like obj-ff-*. Have you tried pointing --with-system-libjpeg directly to the directory with the dylib instead of symlinking it? I'm out of town or I'd try this myself right now.

The lib has to reside in a sub-directory "lib" inside the path passed to "--with-system-libjpeg" and as well the headers but in a sub-directory "include".

All that can be worked around by making the build system place the headers and shared library or symlinks to them in the directories "obj-ff-/dist/include" or "obj-ff-/dist/lib" respectively.

Now a reworked version that does the necessary verification for null pointers in an inlined function in the jpeglib header. It does verify for encoding as well as for decoding although encoding remains untested because I don't know where it is used in firefox.

It doesn't need the symlink to the libjpeg any longer but just the headers.

You activate it by adding just "ac_add_options --with-system-jpeg" without a path to your mozconfig.

I experience the issue that with certain large jpeg images there is a long delay (a minute or so here) after the first lines of image data are decoded.

Here a release candidate version of the patch.

The decoder now works for the files that previously didn't load correctly. The reason is basically an error in the mozilla sources because it sometimes passes a non-zero number of bytes available in the buffer while the buffer pointer indeed is a NULL pointer. This gets solved by resetting the number of bytes in the buffer to zero and calling the mozilla JPEG decoder to supply us with new data. Sounds logical and works!

I tested the JPEG encoder as well using a screenshot grabbing add-on. Using a breakpoint I verified it indeed uses the apple libjpeg.

So now I don't know of any more issues with that patch.

Looks good. Mozilla has signed off on 7, so I'm already building it, but this will be in 7.0.1 or 8b1.

Promoting you to contributor so I can credit you for the issue internally.

Some simple speed comparisons done on a PowerBook Wallstreet G4 500 MHz:

using djpeg to convert a jpeg file (photo, 9.4 MB) to ppm format, destination /dev/null (default options):

libjpeg ImageIO

real 0m1.478s user 0m1.226s sys 0m0.148s

libjpeg 6b

real 0m3.979s user 0m3.708s sys 0m0.159s

libjpeg 8c

real 0m4.251s user 0m3.967s sys 0m0.173s

using cjpeg to convert the result from above (43 MB) back to jpeg format, destination /dev/null (default options):

libjpeg ImageIO

real 0m3.314s user 0m2.762s sys 0m0.479s

libjpeg 6b

real 0m3.668s user 0m3.077s sys 0m0.473s

libjpeg 8c

real 0m3.663s user 0m3.084s sys 0m0.472s

libjpeg 6b was built using gcc-4.6 with LTO and auto-vectorization enabled. cjpeg and djpeg to link against the ImageIO libJPEG were built with the same settings. libjpeg 8c was built by MacPorts.

The decompression speed is impressively improved while there's little improvement in compressing. I did this speed tests as measuring the improvement inside TFF isn't that trivial.

Are you planning to use ImageIO for PNG decoding too?

For PNG, never, as the libpng in 10.4 ImageIO has several known vulnerabilities. So we will only ever use our in-tree png (which doesn't have any form of SIMD acceleration anyway, so we're not losing anything; see M655693 and others).

To the best of my audit ability, the JPEG code does not (there was an exploit fixed in 10.4.6, and I have not been able to find a vulnerable exploit since), is the easiest to disable if one is found and has the most to gain from imageio, so we're going to do so for JPEG only right now. Other image types might be considered in the future, but PNG won't be one of them.

Already thought of it but PNG decoding isn't altivec accelerated in ImageIO. TIFF and JPEG2000 are accelerated.

Scheduled for 8

I landed this, but it isn't linking against libJPEG.dylib. What does otool -L say on your XUL?

I'm still fiddling with it.

You don't need to fiddle - must be some simple issue.

You added "--with-system-jpeg" (WITHOUT any path argument!!!) to your mozconfig and reconfigured and rebuilt everything?

After configuring the source tree ("make -f client.mk configure") there must be the following lines in "config.status" in your build directory: "s%@JPEG_CFLAGS@%-I/Volumes/Programme/OpenSource/mozilla-beta/jpeg-imageio%g s%@JPEG_LIBS@%-L/System/Library/Frameworks/ApplicationServices.framework/Frameworks/ImageIO.framework/Resources -lJPEG%g"

Yes, I'm not wholly incompetent as a project lead ;)

I don't see it linked to XUL or in the config.status, so something is wrong on this end. I'm rechecking the diff to make sure it landed right.

No errors in "config.log" either?

Still working fine here.

configure must output the line: "checking for _cg_jpeg_destroy_compress in -lJPEG... yes"

You might actually have to delete "config.cache" in your build directory before reconfiguring (not to forget running "autoconf213" first).

Or just remove the lines: "ac_cv_lib_jpeg_jpeg_destroy_compress=${ac_cv_lib_jpeg_jpeg_destroy_compress=no}" and "ac_cv_lib_JPEG__cg_jpeg_destroy_compress=${ac_cv_lib_JPEG__cg_jpeg_destroy_compress=no}" from config.cache.

I see that configure line, but not the line in config.status. I'm going to try blasting everything out and rebuilding from the diffs. It is undoubtedly something stuck in the cache.

A slight tweak to configure.in was needed for 8. I'm waiting for it to rebuild and I'll test it tonight (I have a meeting shortly which will take up the rest of the day).

Landed in 8 beta. Overall improvement approximately 1.25x. Job well done.

configure.in has to be tweaked once again for 16. Attached comes an updated patch (from my mercurial queue).

There was a bug in the previous patch so here a patch to the patch:

diff -r 2887b2e42a95 configure.in --- a/configure.in Mon Jul 23 13:19:43 2012 +0200 +++ b/configure.in Fri Jul 27 16:11:15 2012 +0200 @@ -4044,7 +4044,7 @@ dnl === TenFourFox issue 51 if test "$MOZ_NATIVE_JPEG" = 1 -a "${JPEG_DIR}" = "/System/Library/Frameworks/ApplicationServices.framework/Frameworks/ImageIO.framework/Resources"; then MOZ_JPEG_CFLAGS="-I${topsrcdir}/jpeg-imageio" - MOZ_JPEG_LIBS="-L{JPEG_DIR} ${MOZ_JPEG_LIBS}" + MOZ_JPEG_LIBS="-L${JPEG_DIR} ${MOZ_JPEG_LIBS}" else

if test -n "${JPEG_DIR}" -a -d "${JPEG_DIR}" -a "$MOZ_NATIVE_JPEG" = 1; then

Working on 17. I assume the configure.in is the only thing that needs to be changed for 16-17.

In Aurora 18 a big number of JPEGs is displayed white.

This could be bug 792199. It is being backed out.

Might also be that ImageIO libjpeg doesn't (fully) support JCS which was introduced with bug 791305.

It was indeed some JCS extension. I managed to make some changes that simply don't use the additional JCS extensions the ImageIO libjpeg doesn't support. I also downgraded the libjpeg headers to that of version 6b (alias 62).

Attached comes a patch that has to be applied on top of the already existing changes.

Reopened for 18

Landed in 10.4Fx 19 tree

Verified working