GNOME Bugzilla – Bug 469901
Add JPEG 2000 support
Last modified: 2010-07-10 04:06:37 UTC
It seems pretty common to use libjasper for this purpose: http://www.ece.uvic.ca/~mdadams/jasper/
*** Bug 469904 has been marked as a duplicate of this bug. ***
Created attachment 95973 [details] [review] gtk-jasper-draft.patch First pass at a patch. I managed to display something, but I'm sure it's treading on the stack.
Created attachment 96123 [details] [review] gtk-jasper-1.patch Still needs cleaning up. Changes: - some Jasper functions swap x/y, especially matrix functions - handle grayscale images There's no support for YCbCr images, unfortunately.
Created attachment 96146 [details] [review] gtk-jasper-2.patch Fix calls to _set_size clipping the image, and cleanups. Should be committable, although you'll notice it doesn't work with eog (because it expects size_prepared to be called at the beginning of the file, and doesn't check for gdk_pixbuf_loader_close's retval).
Created attachment 96147 [details] [review] gtk-jasper-3.patch Fix handling of images with an opacity channel (such as the data contained in the MacOS X icons in bug 395738), fix a C-99 declaration.
Created attachment 96148 [details] [review] gtk-jasper-4.patch Add j2k as a suffix, and fix JPC detection (for items at http://www.openjpeg.org/samples/) We can even read the humoungous map linked in from the samples: http://memory.loc.gov/ammem/help/view.html
(In reply to comment #4) > > Should be committable, although you'll notice it doesn't work with eog (because > it expects size_prepared to be called at the beginning of the file, and doesn't > check for gdk_pixbuf_loader_close's retval). > JFYI, I committed a fix to eog's SVN today which should make it work with such late-emitting launchers (bug #482128). Loader seems to work fine with my own quickly created test photos.
While we generally discourage adding new loaders (in particular those pulling in new library dependencies) to gdk-pixbuf itself, I can see the point of this being useful to make the icns loader work better, and thus ultimatively the os x port. The code looks ok in general, just a few small things: + if (bits_per_sample < 8) + bits_per_sample = 8; + else if (bits_per_sample > 8) + bits_per_sample = 16; gdk-pixbuf only supports 8, so passing in 16 here is pretty pointless. + context->pixbuf = gdk_pixbuf_new (GDK_COLORSPACE_RGB, + FALSE, bits_per_sample, + context->width, context->height); In the name of OOM handling, this should probably use g_try_malloc() and gdk_pixbuf_new_from_data(). + static gchar *extensions[] = { + "jpc", + "jp2", + "j2k", + NULL + }; This should probably be kept in sync with shared-mime-info, ie add the alternative extensions to the glob pattern there, too. If clean up these issues, feel free to commit this.
(In reply to comment #8) > While we generally discourage adding new loaders (in particular those pulling > in new library dependencies) to gdk-pixbuf itself, I can see the point of this > being useful to make the icns loader work better, and thus ultimatively the os > x port. > > The code looks ok in general, just a few small things: > > + if (bits_per_sample < 8) > + bits_per_sample = 8; > + else if (bits_per_sample > 8) > + bits_per_sample = 16; > > gdk-pixbuf only supports 8, so passing in 16 here is pretty pointless. Does it hurt in any way? I'd rather leave it as-is, as JPEG 2000 allows for higher bpp, which it would be a shame to miss if we were ever to support 16bpp. > + context->pixbuf = gdk_pixbuf_new (GDK_COLORSPACE_RGB, > + FALSE, bits_per_sample, > + context->width, > context->height); > > In the name of OOM handling, this should probably use g_try_malloc() and > gdk_pixbuf_new_from_data(). Done. > + static gchar *extensions[] = { > + "jpc", > + "jp2", > + "j2k", > + NULL > + }; > > This should probably be kept in sync with shared-mime-info, ie add the > alternative extensions to the glob pattern there, too. Done, as well as the mime-types.
Created attachment 99428 [details] [review] gtk-jasper-5.patch Updated patch from above comments
> Does it hurt in any way? I'd rather leave it as-is, as JPEG 2000 allows for > higher bpp, which it would be a shame to miss if we were ever to support 16bpp. I don't think support for 16bpp is going to arrive any time soon. Anyway, if you do pass in 16, gdk_pixbuf_new will fail and return NULL, and I don't think your code handles that right now. If you really want to leave the 16bpp in there, please add a NULL check for context->pixbuf. Other than that, the patch is fine to commit.
I've disabled the 16bpp support, and we force downsampling to 8bpp now. 2007-11-25 Bastien Nocera <hadess@hadess.net> * configure.in: Add detection for libjasper, used by the gdk-pixbuf JPEG2000 loader 2007-11-25 Bastien Nocera <hadess@hadess.net> * Makefile.am: * io-jasper.c: Add the libjasper JPEG2000 loader (Closes: #469901)