After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 588740 - JPEG images open with blur effect?
JPEG images open with blur effect?
Status: RESOLVED FIXED
Product: gdk-pixbuf
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
: 592264 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2009-07-16 05:53 UTC by denvist
Modified: 2010-07-10 04:09 UTC
See Also:
GNOME target: ---
GNOME version: 2.25/2.26


Attachments
fir for loader with jpeg7 (506 bytes, patch)
2009-07-18 07:30 UTC, Gerardo Exequiel Pozzi
none Details | Review
gtk+-2.16.5-jpeg-backward-compatibility.patch (870 bytes, patch)
2009-09-01 10:36 UTC, Romain Perier
none Details | Review

Description denvist 2009-07-16 05:53:30 UTC
After update Eog from 2.26.2 to 2.26.3 version JPEG images look like with blur effect.
Look at the picture: http://imgur.com/Gro3P.jpg

OS: Archlinux

P.S.: Sorry for my bad english :)
Comment 1 Jan de Groot 2009-07-16 08:44:02 UTC
This is not caused by eog. Eog was updated for the rebuild for libjpeg7. This problem affects evolution, eog, geeqie and maybe others. I think the gdk-pixbuf loader is used by these applications.
Comment 2 Matthias Clasen 2009-07-17 03:39:40 UTC
Sounds like a compatibility problem in libjpeg to me, without looking closer.
Comment 3 denvist 2009-07-17 05:38:06 UTC
These programs & libraries have been updated yesterday:
libjpeg, libtiff, libcups, ghostscript, jasper, lcms, libwmf, imagemagick, eog, libdjvu, gd, gtk2, gegl, libmng, imlib2, libgdiplus, libgphoto2, libwebkit, mjpegtools, poppler, sdl_image
Comment 4 Claudio Saavedra 2009-07-17 07:46:30 UTC
Could you please specify the context on which these modules were "updated yesterday"? I am not understanding what you mean.
Comment 5 Jan de Groot 2009-07-17 08:09:52 UTC
Those packages were updated in the archlinux distribution. Last weekend we finished the libjpeg7 migration, which caused updates for the packages named by denvist (and many more not named here).

I haven't seen this blur effect with other applications than those using gdk-pixbuf. These blurred images show up in thumbnails, pidgin avatars, eog, geeqie, evolution attachments, etc, so I think it's a combination of changes in libjpeg7 and the way gtk uses it.
Comment 6 Matthias Clasen 2009-07-17 14:33:45 UTC
Possible. I may have to debug this when libjpeg7 arrives on my system...
Comment 7 Gerardo Exequiel Pozzi 2009-07-18 07:30:32 UTC
Created attachment 138653 [details] [review]
fir for loader with jpeg7

Old libjpeg6b doc says:
unsigned int scale_num, scale_denom
        Scale the image by the fraction scale_num/scale_denom.  Default is
        1/1, or no scaling.  Currently, the only supported scaling ratios
        are 1/1, 1/2, 1/4, and 1/8.  (The library design allows for arbitrary
        scaling ratios but this is not likely to be implemented any time soon.)
        Smaller scaling ratios permit significantly faster decoding since
        fewer pixels need be processed and a simpler IDCT method can be used.

And the new libjpeg7 says:
unsigned int scale_num, scale_denom
        Scale the image by the fraction scale_num/scale_denom.  Default is
        1/1, or no scaling.  Currently, the supported scaling ratios are
        8/N with all N from 1 to 16.  (The library design allows for arbitrary
        scaling ratios but this is not likely to be implemented any time soon.)
Comment 8 denvist 2009-07-20 05:40:02 UTC
Thanx! After the upgrade everything works fine :)
Comment 9 Götz Waschk 2009-08-19 07:52:23 UTC
*** Bug 592264 has been marked as a duplicate of this bug. ***
Comment 10 Götz Waschk 2009-08-19 07:52:54 UTC
to #8: after what update?
Comment 11 denvist 2009-08-19 08:10:44 UTC
(In reply to comment #10)
> to #8: after what update?

Problem was in gtk2 package.
See message "Gerardo Exequiel Pozzi 2009-07-18 07:30:32 UTC"
Comment 12 Paul Bredbury 2009-08-26 04:16:36 UTC
The thread being referred to is http://bbs.archlinux.org/viewtopic.php?id=75529

Notice improved patch by putte_xvi:
http://bbs.archlinux.org/viewtopic.php?pid=589488#p589488
Comment 13 Romain Perier 2009-08-26 13:42:53 UTC
@Matthias : We're interested about this bug in order to commit the fix into the gentoo main tree, can you confirm this patch ? (Officially I meant)

thanks :)
Comment 14 moof 2009-08-29 05:51:45 UTC
I can confirm that putte_xvi's patch works on NetBSD, restoring the expected behavior.
Comment 15 Matthias Clasen 2009-08-30 02:11:41 UTC
It sounds like libjpeg7 is simply incompatible with libjpeg6 as far as this scaling feature goes. The patch might work for libjpeg7, but ideally, we need something that works with both versions of libjpeg.
Comment 16 Romain Perier 2009-08-30 10:15:27 UTC
Okay that I wanted to hear, but I had a doubt so I prefered to ask you ;)
Comment 17 Romain Perier 2009-09-01 10:35:30 UTC
Does the patch in attachment look correct ? I simply used the conditionnal inclusion using JPEG_LIB_VERSION in order to preserve backward compatibility.

@Matthias: May be it's not perfect, in this case could you explain me what's wrong ? :)
Comment 18 Romain Perier 2009-09-01 10:36:15 UTC
Created attachment 142226 [details] [review]
gtk+-2.16.5-jpeg-backward-compatibility.patch
Comment 19 Tor Lillqvist 2009-09-01 10:41:14 UTC
But is a compile-time check for libjpeg version good enough, if the intent of the libjpeg maintainers is that the libjpeg versions are binary compatible and are built so that the soname / DLL name of version 7 is the same as of version 6? Would a run-time check be better? But is there any API for that...?
Comment 20 Nirbheek Chauhan 2009-09-01 11:49:28 UTC
(In reply to comment #19)
> But is a compile-time check for libjpeg version good enough, if the intent of
> the libjpeg maintainers is that the libjpeg versions are binary compatible and
> are built so that the soname / DLL name of version 7 is the same as of version
> 6? Would a run-time check be better? But is there any API for that...?

libjpeg 6 and 7 are not API/ABI compatible. Hence there is an soname change.
Comment 21 Romain Perier 2009-09-01 12:08:52 UTC
Tor:  soname for jpeg6 and jpeg7 are differents.

with jpeg-7 (on my gentoo unstable):
$ readelf -d /usr/lib/libjpeg.so | grep SONAME
 0x000000000000000e (SONAME)             Library soname: [libjpeg.so.7]

with jpeg-6b (on my gentoo stable):
$ readelf -d /usr/lib/libjpeg.so | grep SONAME
 0x000000000000000e (SONAME)             Library soname: [libjpeg.so.62]

in other words, If you build gtk+ using jpeg-7, then you downgrade jpeg-7 to jpeg-6b, libpixbufloader-jpeg.so will be still "depends" on libjpeg.so.7 soname in its DT_NEEDED entry.
Comment 22 Tor Lillqvist 2009-09-01 14:45:56 UTC
ok, thanks, I was misremembering that they would be claimed to be ABI compatible.
Comment 23 Romain Perier 2009-09-02 19:39:56 UTC
Matthias : Can I have your opinion about the fix ? 
thanks in advance
Comment 24 Allan 2009-10-08 09:02:28 UTC
This is a comment from Guido Vollbeding, organizer Independent JPEG Group:

I have now looked through the code and found the flawed code sequence
in function "gdk_pixbuf__jpeg_image_load_increment".
I strongly advise to correct the code by simply inserting the statement

	cinfo->scale_num = 1;

in front of the following code sequence in mentioned function:

	for (cinfo->scale_denom = 2; cinfo->scale_denom <= 8; cinfo->scale_denom *= 2) {
		jpeg_calc_output_dimensions (cinfo);
		if (cinfo->output_width < width || cinfo->output_height < height) {
			cinfo->scale_denom /= 2;
			break;
		}
	}
	jpeg_calc_output_dimensions (cinfo);

This addition will work with older and newer versions of the library.

The implicit assumption of the given code that cinfo->scale_num is
initialized with 1 by the JPEG library is no longer true for versions
7 and later!  Version 7 initializes this field (and the other) with 8,
and version 8 and later will initialize the fields with the (variable
from 1 to 16) block size of the given JPEG file.  (Note that the
default resulting scaling factor remains 1 in any case.)

The usual recommendation for versions up to 6 has always been that
"scale_num" and "scale_denom" be set *simultaneously* by the calling
application.  Applications following this recommendation will not
suffer an incompatibility with newer JPEG library versions.
Newer applications (written for JPEG library versions 7 and later)
MAY choose to set only one of both fields, because the initialization
defaults are now depending on the input file and are specified as
mentioned above.

The given correction code simply retains the same behaviour with
new JPEG library versions as with old JPEG library versions.
The code may later be revised to utilize the more flexible scaling
choices of v7 and later, but there is no need to do this now.
Comment 25 Matthias Clasen 2009-11-05 15:38:28 UTC
I've committed a fix along those lines to master and gtk-2-18.
Could someone using jpeg7 please verify that this indeed fixes the issue ?
Thanks.
Comment 26 Jan de Groot 2009-11-05 15:45:43 UTC
On Archlinux we placed the extra line before the for loop, not inside. I don't know if the code inside the for-loop touches scale_num, but if it does, this results in bad behaviour still.
Comment 27 Matthias Clasen 2009-11-05 15:58:49 UTC
I placed it inside the loop, to follow this comment:

The usual recommendation for versions up to 6 has always been that
"scale_num" and "scale_denom" be set *simultaneously* by the calling
application. 

Since we are setting denom in the loop, I figured that we should set num as well.
Comment 28 Guido Vollbeding 2009-11-15 09:51:44 UTC
"Simultaneously" means setting both fields before calling the JPEG library function (jpeg_start_decompress or jpeg_calc_output_dimensions here, which is where these fields are processed by the library).

So placing the extra line before the for loop in the application code is correct.

Regards
Guido Vollbeding
Organizer Independent JPEG Group
Comment 29 Guido Vollbeding 2009-11-15 10:10:24 UTC
Note that the statement given in Comment 7 is confused.

The 8/N scaling is for *compression*, which is new in JPEG 7!

For decompression it is N/8, and the old cases are a subset thereof (N=8,4,2,1).

Regards
Guido Vollbeding
Organizer Independent JPEG Group
Comment 30 Matthias Clasen 2009-11-26 04:44:04 UTC
(In reply to comment #28)
> "Simultaneously" means setting both fields before calling the JPEG library
> function (jpeg_start_decompress or jpeg_calc_output_dimensions here, which is
> where these fields are processed by the library).
> 
> So placing the extra line before the for loop in the application code is
> correct.

I've done this now.