Use Apple jpeg-imageio for AltiVec JPEG

[GoogleCodeExporter]

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

Original issue reported on code.google.com by classi...@floodgap.com on 3 Apr 2011 at 10:25

• Blocking: #73, #73
• Blocked on: #28

[GoogleCodeExporter]

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

Original comment by classi...@floodgap.com on 7 Jun 2011 at 5:35

• Removed labels: Milestone-NextBeta5

[GoogleCodeExporter]

Original comment by classi...@floodgap.com on 21 Jun 2011 at 10:24

[GoogleCodeExporter]

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.

Original comment by Tobias.N...@gmail.com on 12 Sep 2011 at 9:51

[GoogleCodeExporter]

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.

Original comment by Tobias.N...@gmail.com on 12 Sep 2011 at 9:51

[GoogleCodeExporter]

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-Marc
h/002398.html

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

Original comment by Tobias.N...@gmail.com on 13 Sep 2011 at 9:06

[GoogleCodeExporter]

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...

Original comment by Tobias.N...@gmail.com on 13 Sep 2011 at 4:43

[GoogleCodeExporter]

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.

Original comment by classi...@floodgap.com on 13 Sep 2011 at 5:27

[GoogleCodeExporter]

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.

Original comment by Tobias.N...@gmail.com on 13 Sep 2011 at 7:03

[GoogleCodeExporter]

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.

Original comment by Tobias.N...@gmail.com on 13 Sep 2011 at 7:08

[GoogleCodeExporter]

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%.

Original comment by Tobias.N...@gmail.com on 13 Sep 2011 at 11:05

Attachments:

• jpeg-imageio.patch

[GoogleCodeExporter]

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?)

Original comment by Tobias.N...@gmail.com on 14 Sep 2011 at 7:34

[GoogleCodeExporter]

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.

Original comment by classi...@floodgap.com on 14 Sep 2011 at 6:52

[GoogleCodeExporter]

Original comment by classi...@floodgap.com on 14 Sep 2011 at 6:52

• Changed state: Started

[GoogleCodeExporter]

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.

Original comment by classi...@floodgap.com on 14 Sep 2011 at 6:54

[GoogleCodeExporter]

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.

Original comment by Tobias.N...@gmail.com on 18 Sep 2011 at 6:22

[GoogleCodeExporter]

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.

Original comment by classi...@floodgap.com on 19 Sep 2011 at 1:37

[GoogleCodeExporter]

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.

Original comment by Tobias.N...@gmail.com on 19 Sep 2011 at 4:32

[GoogleCodeExporter]

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.

Original comment by Tobias.N...@gmail.com on 19 Sep 2011 at 9:20

Attachments:

• jpeg-imageio_2.patch

[GoogleCodeExporter]

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.

Original comment by Tobias.N...@gmail.com on 19 Sep 2011 at 9:33

[GoogleCodeExporter]

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.

Original comment by Tobias.N...@gmail.com on 21 Sep 2011 at 8:46

Attachments:

• jpeg-imageio_3.patch

[GoogleCodeExporter]

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

Original comment by classi...@floodgap.com on 22 Sep 2011 at 3:30

[GoogleCodeExporter]

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

Original comment by classi...@floodgap.com on 22 Sep 2011 at 8:26

• Changed title: Use Apple jpeg-imageio for AltiVec JPEG

[GoogleCodeExporter]

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.

Original comment by Tobias.N...@gmail.com on 25 Sep 2011 at 6:47

[GoogleCodeExporter]

Are you planning to use ImageIO for PNG decoding too?

Original comment by annu...@gmail.com on 26 Sep 2011 at 8:54

[GoogleCodeExporter]

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.

Original comment by classi...@floodgap.com on 26 Sep 2011 at 2:56

[GoogleCodeExporter]

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

Original comment by Tobias.N...@gmail.com on 26 Sep 2011 at 6:27

[GoogleCodeExporter]

Scheduled for 8

Original comment by classi...@floodgap.com on 27 Sep 2011 at 4:19

• Added labels: Milestone-NextBeta8, Priority-High
• Removed labels: Priority-Medium

[GoogleCodeExporter]

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.

Original comment by classi...@floodgap.com on 4 Oct 2011 at 1:19

[GoogleCodeExporter]

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/Framewo
rks/ImageIO.framework/Resources -lJPEG%g"

Original comment by Tobias.N...@gmail.com on 4 Oct 2011 at 3:59

[GoogleCodeExporter]

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.

Original comment by classi...@floodgap.com on 4 Oct 2011 at 4:05

[GoogleCodeExporter]

No errors in "config.log" either?

Original comment by Tobias.N...@gmail.com on 4 Oct 2011 at 4:08

[GoogleCodeExporter]

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).

Original comment by Tobias.N...@gmail.com on 4 Oct 2011 at 4:33

[GoogleCodeExporter]

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_comp
ress=no}"
from config.cache.

Original comment by Tobias.N...@gmail.com on 4 Oct 2011 at 4:36

[GoogleCodeExporter]

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.

Original comment by classi...@floodgap.com on 4 Oct 2011 at 4:36

[GoogleCodeExporter]

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).

Original comment by classi...@floodgap.com on 4 Oct 2011 at 5:25

[GoogleCodeExporter]

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

Original comment by classi...@floodgap.com on 8 Oct 2011 at 1:22

• Changed state: Verified

[GoogleCodeExporter]

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

Original comment by Tobias.N...@gmail.com on 16 Jul 2012 at 10:39

• Now blocked on: #28

Attachments:

• make_possible_the_use_of_system_libjpeg_on_mac_os_x.patch

[GoogleCodeExporter]

Original comment by classi...@floodgap.com on 23 Jul 2012 at 1:47

• Now blocking: #73

[GoogleCodeExporter]

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

Original comment by Tobias.N...@gmail.com on 27 Jul 2012 at 2:14

[GoogleCodeExporter]

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

Original comment by classi...@floodgap.com on 15 Sep 2012 at 5:30

[GoogleCodeExporter]

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

Original comment by Tobias.N...@gmail.com on 12 Oct 2012 at 3:49

[GoogleCodeExporter]

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

Original comment by classi...@floodgap.com on 12 Oct 2012 at 5:24

[GoogleCodeExporter]

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

Original comment by Tobias.N...@gmail.com on 12 Oct 2012 at 5:59

[GoogleCodeExporter]

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.

Original comment by Tobias.N...@gmail.com on 12 Oct 2012 at 6:51

Attachments:

• jpeg-imageio.diff

[GoogleCodeExporter]

Reopened for 18

Original comment by classi...@floodgap.com on 12 Oct 2012 at 8:51

• Changed state: Reopened
• Added labels: Milestone-Judgment-Day
• Removed labels: Milestone-NextBeta8

[GoogleCodeExporter]

Landed in 10.4Fx 19 tree

Original comment by classi...@floodgap.com on 3 Dec 2012 at 5:10

• Changed state: Started

[GoogleCodeExporter]

Verified working

Original comment by classi...@floodgap.com on 21 Dec 2012 at 4:44

• Changed state: Verified