Opened 12 years ago

Last modified 7 weeks ago

#7344 closed enhancement

New libjpeg package — at Version 13

Reported by: timdumol Owned by: mabshoff
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: packages: optional Keywords: sd32
Cc: david.kirkby@…, kcrisman, leif Merged in:
Authors: Tim Dumol Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by timdumol)

This is used by PIL (c.f. #7273). Inclusion as an optional or even as a standard package would be helpful.

The latest version of the package is here: http://sage.math.washington.edu/home/timdumol/libjpeg-7.p2.spkg

Change History (14)

comment:1 Changed 12 years ago by timdumol

  • Status changed from new to needs_review

comment:2 Changed 12 years ago by mhampton

There is an extra file, SPKG.txt~, in the package.

I don't know if this should block its inclusion (I think this should go in as an optional package ASAP so it gets wider testing) but the show function doesn't work on my intel mac, 10.4.11, opening a TIFF file:

sage: import Image
sage: im = Image.open("/Users/mh/Pictures/psym.tiff")
sage: print im.format, im.size, im.mode
TIFF (455, 495) 1
sage: im.show()
sage: dyld: Symbol not found: __cg_jpeg_resync_to_restart
  Referenced from: /System/Library/Frameworks/ApplicationServices.framework/Versions/A/Frameworks/ImageIO.framework/Versions/A/ImageIO
  Expected in: /Volumes/E/sage-4.2.1/local/lib//libJPEG.dylib

comment:3 Changed 12 years ago by mhampton

Oops that was a TIFF example, but it happens with jpegs too:

sage: im = Image.open("/Users/mh/Pictures/v2shot.jpg")
sage: im.show()
sage: dyld: Symbol not found: __cg_jpeg_resync_to_restart
  Referenced from: /System/Library/Frameworks/ApplicationServices.framework/Versions/A/Frameworks/ImageIO.framework/Versions/A/ImageIO
  Expected in: /Volumes/E/sage-4.2.1/local/lib//libJPEG.dylib

comment:4 Changed 12 years ago by timdumol

The error seems to be an issue with having multiple versions of libjpeg in OS X. Refer to: http://old.nabble.com/dyld:-Symbol-not-found:-__cg_jpeg_resync_to_restart-to9949988.html. I am unsure as to how to fix that problem (I haven ever used a Mac).

comment:5 Changed 12 years ago by timdumol

  • Description modified (diff)

A new version without SPKG.txt~ is up at http://sage.math.washington.edu/home/timdumol/libjpeg-7.p1.spkg.

comment:6 Changed 12 years ago by drkirkby

  • Cc david.kirkby@… added
  • Report Upstream set to N/A

This needs checking on Solaris

comment:7 Changed 12 years ago by jhpalmieri

When I try an example like mhampton's above (im = Image.open(...), im.show(), I get the message

ImportError: The _imaging C module is not installed

(This is on OS X 10.6.) Is this an issue?

comment:8 Changed 12 years ago by drkirkby

Sorry, I did not set up email notifications until recently, so cc'ing me did not help. I only stumbled across this when looking for tickets to review. I will check it on Solaris.

Dave

comment:9 follow-up: Changed 12 years ago by drkirkby

  • Status changed from needs_review to needs_work

A few points looking at spkg-install, which is overly complicated following recent updates to sage-env.

  • The recent changes to sage-env (#7818) will mean a lot of spkg-install can be removed. In particular most, if not all the SAGE64 stuff, as sage-env will automatically add the right flags to CFLAGS, CXFLAGS etc for a 64-bit build. It will not do this (yet) for LDFLAGS so it may be necessary to add that, but I'm not convinced it will be necessary. It needs testing on bsd.math without the LD flags set. If it builds, then forgot it all. If it needs LDFLAGS to have -m64, replace -m64 with
    if [ "x$SAGE64" = xyes ] ; then
      LDFLAGS="$LDFLAGS $CFLAG64" 
      export $LDFLAGS
    fi
    

then it will work irrespective of the flag needed to generate 64-bit binaries. (Not all compilers need -m64, as others use -q64 and other flags). I'm not totally convinced LDFLAGS ever needs -m64, as I believe LDFLAGS gets passed to the linker. No linker I am aware of has the -m64 option (check GNU binutils manual if you wish). But I'm not 100% sure it is never needed. I think the right flag if needed is -64 for the GNU linker, but it is rarely needed, as normally the linker can work out whether a 32 or 64 bit binary is needed, based on the object files. The exception is when creasting shared libraries from archives, where the linker may not be able to determine this.

  • Remove the checks for the Sun compiler and adding -Wall with gcc. The updates to sage-env will add the -Wall for you with gcc.
  • Remove the checks to see if there is a mix of Sun and GNU compilers - that is taken care of elsewhere now.
  • Unless there is good reason, remove the
: ${CP=cp}; CP="$CP -f"; export CP
: ${MV=mv}; MV="$MV -f"; export MV
: ${RM=rm}; RM="$RM -f"; export RM

as it was agreed recently (see sage-devel) that there is no need for there to be variables for very basic commands. However, if the source code makes use of $CP etc, then they might have to stay. But if possible, just use mv, cp etc.

  • Someting called 'CC' is used here. I'm not sure what that is, but be aware the Sun C++ compiler is called 'CC' so there might be a name clash. Make sure the absolute path to CC is specified if it's not already done, otherwise it may break if the Sun C++ compiler is in the path.
  • Give me a day or so, and I'll put on sage-devel a considerably simpler skeleton spkg-install which will remove 95% of this.

It does actually build on Solaris, but the spkg-install could be reduced considerably in size, as most of these checks are now done in once place.

I'll help you do this.

Dave

comment:10 in reply to: ↑ 9 Changed 12 years ago by timdumol

Replying to drkirkby:

[...]

  • Give me a day or so, and I'll put on sage-devel a considerably simpler skeleton spkg-install which will remove 95% of this.

It does actually build on Solaris, but the spkg-install could be reduced considerably in size, as most of these checks are now done in once place.

I'll help you do this.

Dave

Perfect, thanks. The libjpeg makefile actually uses $CP, $RM and $MV, so that section can't be removed.

Changed 12 years ago by drkirkby

comment:11 Changed 12 years ago by drkirkby

I've looked at this, and have attached a revised spkg-install, which is a lot simpler. However, there are some odd things about this.

  • Why is the package called libjpeg-7.p1, rather than libjpeg-7 ? The .p0 is appended when a patch is applied, .p1 is used when a second patch is applied. This would therefore imply it's been patched twice, whereas in fact it has not.
  • There is no need for a patches directory when there are no patches.
  • What is the purpose of this code
: ${CP=cp}; CP="$CP -f"; export CP
: ${MV=mv}; MV="$MV -f"; export MV
: ${RM=rm}; RM="$RM -f"; export RM

I just took the libjpeg source code, then built it with:

$ ./configure --prefix=/tmp
$ make 
$ make install

and it all went ok, without me having to override 'cp', 'mv' or 'rm'.

  • You would need to get William or someone else to look over the license. I know there is one requirement there, which is not a requirement of the GPL. That is, if you use their code, you must acknowledge them. Having had some GPL'ed code of mine ripped off without acknowledgment, I was a bit annoyed, but I've been told there is no requirement to acknowledge anyone if you use their GPL code. I am not a lawyer are more interested in the technical aspects than license conditions, but someone would need to verify this license is compatible.

In the attacked in spkg-install I've left all the overriding of cp, but I don't feel it should be necessary.

Dave

comment:12 Changed 11 years ago by kcrisman

See also here.

comment:13 Changed 11 years ago by timdumol

  • Cc kcrisman added
  • Description modified (diff)
  • Status changed from needs_work to needs_review

New package version here: http://sage.math.washington.edu/home/timdumol/libjpeg-7.p2.spkg

As stated in #7818, some makefiles depend on $RM being unset or set to "rm -f". The above code overrides any $RM setting, which otherwise causes compilation failure (at least for me).

I am unsure of the GPL compatibility of libjpeg's licensing, but this message on: http://www.mail-archive.com/mozilla-license@mozilla.org/msg00143.html seems to suggest that it is widely considered GPL compatible. Otherwise, we may want to consider libjpeg-turbo (http://sourceforge.net/projects/libjpeg-turbo/) and patch PIL to use that.

Regarding review of this package:

The package may be reviewed by using the PIL package, as above:

sage: import Image
sage: im = Image.open("<your-jpeg-file-here>")
sage: im.resize((im.size[0]/2,im.size[1]/2))
sage: print im.format, im.size, im.mode
TIFF (455, 495) 1
sage: im.show()
sage: im.save("wherever.jpg")
Note: See TracTickets for help on using tickets.