Bug 246854 - Dolphin fails to show width/height of certain .jpg's
Summary: Dolphin fails to show width/height of certain .jpg's
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: general (show other bugs)
Version: 16.12.2
Platform: Unlisted Binaries Linux
: NOR normal
Target Milestone: ---
Assignee: Peter Penz
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-06 00:35 UTC by Szczepan Hołyszewski
Modified: 2010-10-26 14:35 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.5.2


Attachments
No image dimensions info for this .jpg (10.47 KB, image/jpeg)
2010-08-06 00:35 UTC, Szczepan Hołyszewski
Details
Fixed jpegendanalyzer.cpp (18.37 KB, application/octet-stream)
2010-10-10 23:31 UTC, Szczepan Hołyszewski
Details
Cleanup: remove includes added in the process but no longer needed (18.27 KB, application/octet-stream)
2010-10-24 13:41 UTC, Szczepan Hołyszewski
Details
Diff against svn (3.28 KB, patch)
2010-10-25 10:29 UTC, Szczepan Hołyszewski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Szczepan Hołyszewski 2010-08-06 00:35:50 UTC
Created attachment 49851 [details]
No image dimensions info for this .jpg

Version:           unspecified
OS:                Linux

For certain jpg images Dolphin fails to show width and height information in the information panel.

Attaching an example of problematic image.

Reproducible: Always

Steps to Reproduce:
Hover mouse over the problematic image.

Actual Results:  
Width and height information should be displayed in the information panel.

Expected Results:  
Width and height information is not displayed in the information panel.
Comment 1 Szczepan Hołyszewski 2010-08-06 00:37:33 UTC
Forgot to set KDE version for this report, it's 4.4.95 (4.5 RC3).
Comment 2 Szczepan Hołyszewski 2010-08-06 00:42:53 UTC
Btw. dolphin shows correct preview of this image, Gwenview opens it just fine etc.
Comment 3 Szczepan Hołyszewski 2010-08-15 17:55:12 UTC
After further investigation it appears that this functionality is completely broken for .jpg images. The information panel shows width and height for some jpegs, fails to show it for others, and shows ***WRONG*** width and height for others yet. The latter happens sometimes when the I resize the images in place using mogrify, but at other times it causes the w&h info to disappear altogether from the information panel.

SHAME!
Comment 4 Szczepan Hołyszewski 2010-09-01 13:56:50 UTC
Bug still present in 4.5.1.
Comment 5 Peter Penz 2010-09-02 20:12:03 UTC
SVN commit 1171107 by ppenz:

Always read the image size of the JPEG from the header, as an available EXIF information does not assure that the image size is stored.

BUG: 246854
FIXED-IN: 4.5.2
CCMAIL: jos@vandenoever.info


 M  +44 -33    jpegendanalyzer.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1171107
Comment 6 Szczepan Hołyszewski 2010-10-06 19:13:21 UTC
NOT FIXED.

Just upgraded to 4.5.2, rebooted just in case, and Dolphin ***STILL*** won't show width/height information for some jpegs, including the one I have attached to this bug report!

Dolphin 1.5/KDE 4.5.2

Reopen please.
Comment 7 Szczepan Hołyszewski 2010-10-10 23:31:05 UTC
Created attachment 52402 [details]
Fixed jpegendanalyzer.cpp

1) No need to dissect jpg header by hand. Use img->pixelWidth() and img->pixelHeight() - these do come from the header, not from EXIF data.

2) Never ever read pixel size from EXIF data. Jpg header should be the only source of this information, not a fallback source.
Comment 8 Szczepan Hołyszewski 2010-10-10 23:33:10 UTC
I think I finally fixed it! Please review my changes and apply if possible.
Comment 9 Peter Penz 2010-10-11 08:31:18 UTC
Thanks for the patch! Could you please post this patch to jos@vandenoever.info for review? He is the maintainer of Strigi and decides whether the patch can go in. I'll apply the patch as soon as we got the OK from Jos.
Comment 10 Szczepan Hołyszewski 2010-10-24 08:13:10 UTC
No response from Jos so far...
Comment 11 Szczepan Hołyszewski 2010-10-24 13:41:22 UTC
Created attachment 52823 [details]
Cleanup: remove includes added in the process but no longer needed
Comment 12 Szczepan Hołyszewski 2010-10-24 13:45:14 UTC
Jos just wrote me back approving the fix. Please apply the cleaned-up version I just attached.
Comment 13 Peter Penz 2010-10-25 09:03:05 UTC
Thanks  Szczepan. It is very uncommon to send the whole files instead of just attaching a patch (see http://techbase.kde.org/Contribute/Send_Patches), as it makes it tricky for the developer to see what has been really changed.

I wanted to check the diff of your patch, but it was very tricky as it seems your editor changed the alignments of a lot of unrelated code (so hundreds of lines haven been marked as "changed"). I then later just applied your patch locally, but it seems not to fix anything that has not been fixed already by the patch applied at http://websvn.kde.org/?view=rev&revision=1171107 (but I might have missed something).
Comment 14 Szczepan Hołyszewski 2010-10-25 09:58:39 UTC
There were two problems with Jos's fix:

- his code reading the jpg header failed on some jpegs

- even if it succeeded, the dimensions from EXIF overrode the dimensions from the header. That wasn't right because EXIF can lie, and often does. Something as common as mogrify -resize on a jpeg with EXIF dimensions will yield a file with false information in EXIF.

I was bitten by both problems while working with photos for websites.

My fix removes Jos's jpeg header dissector (analyzeSize()) and uses Exiv2 API that Jos had overlooked. Furthermore it skips EXIF dimension information altogether - the jpg header is now the only source of this information, not a fallback source.
Comment 15 Szczepan Hołyszewski 2010-10-25 10:29:47 UTC
Created attachment 52846 [details]
Diff against svn

Use svn diff -x -ub to suppress changes that affect only whitespace.
Comment 16 Peter Penz 2010-10-25 10:56:56 UTC
Thanks for the update with the diff, this makes it a lot easier :-) The patch looks good, I'll it try it during this week, will commit it to SVN and set Jos to CC.
Comment 17 Peter Penz 2010-10-26 14:31:55 UTC
SVN commit 1189932 by ppenz:

Fix issue, that the width and height of certain JPG files are not shown. The patch has been written by Szczepan Hołyszewski.

CCMAIL: jos@vandenoever.info
BUG: 246854


 M  +9 -38     jpegendanalyzer.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1189932
Comment 18 Peter Penz 2010-10-26 14:35:20 UTC
I've applied the patch to trunk. kdesupport does not have a separate bugfix-branch for 4.5.x like kdelibs and kdebase and AFAIK updates must be tagged manually to be available for 4.5.x. What I want to say is that I don't know for sure whether fixes like this will be available for 4.5.3 or whether this will be available for 4.6.0.