When using the flip or mirror operation in digikam or gwenview => the entirety of the picture is not mirrored of flipped: there is a little strip left unchanged. See attachments. Reproducible: Always Steps to Reproduce: Happens every time with the picture given as example. (No sure if it happens with any picture, but usually well with little pictures (<100ko)). - Not sure if this is related to the 'JPEGLossLess' kipiplugin but obviously this is not lossless ! - The bug is present with digikam and gwenview (KDE 4.10, but I had already remarked this bug before (at least 1 year ago -> this is not new).
Created attachment 80000 [details] Original picture
Created attachment 80001 [details] Mirrored picture (with gwenview) -> see the left strip
Created attachment 80002 [details] Mirrored picture (with digikam) -> see the left strip (same bug with digikam and gwenview: the mirrored picture is the same, except that digikam adds metedata)
Created attachment 80003 [details] Flipped picture (with digikam) -> see the bottom strip
JPEG lossless operations require the image size to be a multiple of 8. Use lossy operations for other sizes.
This has been discussed in bug #146381 (However, I think the current state can and should be improved.)
OK. After reading the bug #146381, especially the comments 9 and 10, I think the better solution is: 1) if the size of the picture is a multiple of 8 -> no problem: gwenview can rotate the picture as now, losslessly 2) if the size of the picture is not a multiple of 8, gwenview can propose a dialog box with 2 choices: 2a) lossly rotate the picture (and of course inform the user it is lossly) 2b) propose to crop the image to a multiple of 8 and then losslessly rotate it A third choice "rotate losslessly and leave a strip unrotated" must not be proposed as nobody wants such a resulting picture ! 2b solution is the best solution for big pictures (a little crop will have nearly no visible result) while 2a is sometimes the only solution for little pictures => both options must be proposed to the user. PS: can somebody confirm that a picture that is NOT a multiple of 8 cannot be losslessly rotated ? Are there other program that jpegtran that would do that ?
digiKam do not use this plugin since a long time. It's problably not reproducible anymore in digiKam. Can you confirm please ? Gilles Caulier
I'm sorry Gilles, I don't know what plugin Digikam uses instead, but the but bug is always there when I mirror the image. See attached file (the result is the same as comments #2 and #3). I still think the only possibility to solve this bug was given in comment #7.
Created attachment 92549 [details] Same bug -> see right strip :-(
digiKam do not use JPEGLossLess plugin since 3.x version (if i remember). So now, Flip/rotate operation is performed with a dedicated method hard coded in digiKam core. The image from comment #10 have generated with digiKam ? If yes, can you share original to try on my computer ? Gilles
Yes, image from comment #10 has been generated with DK. The original picture is in comment #1. Thanks for investigating...
In digiKam, to rotate/flip the image didi you use the transform action from album gui, or image editor, or a workflow from BQM ? Gilles Caulier
(In reply to Gilles Caulier from comment #13) > In digiKam, to rotate/flip the image didi you use the transform action from > album gui, or image editor, or a workflow from BQM ? I used the option "Flip" > "Horizontally (Ctrl+*)" under the "Image" menu.
yes confirmed. Reproducible here...
In original report, you said that bug is reproducible in Gwenview too. It's always the case ? I think there is a precision lost somewhere to compute image size before transform. Problem is also reproducible with rotation left or right.
Problem is not reproducible with Image editor. Problem is reproducible in BQM. So problem is located in JPEGUtils::JpegRotator, especially in this call : https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/libs/jpegutils/jpegutils.cpp#L413 Gilles Caulier
There is nothing special in digiKam source code to process JPEG transform. All is delegate to libjpeg into this method : https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/libs/jpegutils/jpegutils.cpp#L521 I suspect a bug in libjpeg loosless transform code... Gilles Caulier
Nicofo, Which libjpeg version do you use exactly? Look into Help/Components Info dialog for details. Gilles Caulier
Nicofo, Can you test with another NON KDE program which can perform lossless transform through libjpeg. Perhaps another image viewer from GNome project for ex. Just to see if problem is reproducible. Note : Gwenview us a similar method to process lossless transform than digiKam. It's not the same code, but it use libjpeg in background Gilles Caulier
(In reply to Gilles Caulier from comment #19 > Which libjpeg version do you use exactly? libjpeg 62 (I use an up-to-date version of Fedora 21) (In reply to Gilles Caulier from comment #20 > Can you test with another NON KDE program which can perform lossless transform through libjpeg. Perhaps another image viewer from GNome project for ex. Just to see if problem is reproducible. - I tried with eye of gnome (Eye of GNOME Image Viewer) -> same as digikam: strip left unmirrored - I tried with Shotwell viewer: here there is no problem ! Mirroring, rotating doesn't damage the picture.
Very interresting. So Eye of GNOME as the same problem than digiKam, so probably a dysfunction in libjpeg lossless rotation code. If Shotwell work as expected, there is 2 possibility : JPEG transformation is not lossless (as it doesn't use libjpeg code), or, problem have been fixed in libjpeg code included to Shotwell. Are you sure that Shotwell perform JPEG lossless transformations ? Gilles Caulier
Shotwell is not written in C/C++ (vala source code), and it do not use libjpeg transform code. So this one is not suitable to compare with digiKam about this problem. Anyway, i suspect highly a problem in libjpeg implementation Gilles Caulier
Nicofo, Under linux, we have a tool named JPEGTrans : http://linux.die.net/man/1/jpegtran Which use libjpeg lossless transform code. If you can reproduce the problem with it, it's clear, JPEG library has a bug. Can you perform some test with this CLI tool on your computer ? Gilles Caulier
With jpegtran the problem is present as well. If I run it with the "-perfect" option, it refuses to perform the operation: jpegtran -flip horizontal -perfect montain_ORIG.jpg jpegtran: transformation is not perfect
Well, it's clear, it's definitively a libjpeg problem. Please report this bug as UPSTREAM to libjpeg team (jpegturbo project is used everywhere now under Linux). http://libjpeg-turbo.virtualgl.org/ Gilles Caulier
(In reply to Gilles Caulier from comment #26) > Well, it's clear, it's definitively a libjpeg problem. > > Please report this bug as UPSTREAM to libjpeg team (jpegturbo project is > used everywhere now under Linux). OK, I open that bug on jpegturbo: see https://sourceforge.net/p/libjpeg-turbo/bugs/91/ Summary of that bug: this is NOT a libjpeg problem, it is the normal behaviour and software using libjpeg must know that behaviour to handle this case. An example is also given: > It is incumbent upon user-level software to handle this case. For instance, whenever I try to transform an image using GraphicConverter on my Mac, it will pop up a dialog if the image dimensions don't fall on MCU boundaries, and I can choose from that dialog whether I want to trim the image or transform it using a lossy method (decompress/recompress.) [see full explanation https://sourceforge.net/p/libjpeg-turbo/bugs/91/ ]
Very interresting report from libjpeg team... But they miss something in the response : how to check if the MCU boundaries from a JPEG image is right for lossless transform ? Note switching from lossless to lossy transform is easy to do in digiKam all code are already in place. We just need to know to to perform test in jpegutill.cpp, probably somewhere here : https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/libs/jpegutils/jpegutils.cpp#L413 Giles Caulier
After reading information given by libjpegteam, there is the way to check MCU in digiKam : "When flipping the image, if the vertical image dimension isn't evenly divisible by 8 * the vertical chroma sampling factor, then the bottom-most MCU row is incomplete." The question is : how to get vertical chroma sampling factor from image ? Gilles
Nicofo, In comment #7 you said that Gwenview has relevant code to detect if image can be rotated lostless or not and provide relevant option to wrap around. I check Gwenview code (frameworks) : there is no kind of code like this. I check KDE4 version to process your test image : no dialog to handle wrong image size before to process lossless transform. Which king of dialog do you see on screen ? Can you take a screenshot please ? Gilles Caulier
(In reply to Gilles Caulier from comment #30) > In comment #7 you said that Gwenview has relevant code to detect if image > can be rotated lostless or not and provide relevant option to wrap around. No, I was just explaining what a possible solution could be, no what already exists ;)
To Christoph Feck, comment #5, Did you know an implementation which do the stuff already somewhere ? As i explain in comment #29, following tips from libjpeg team, we need to handle chroma sampling factor to check if JPEG can be processed lossless. If we only check if image size is divisible by 8 is not enough. Nicofo, I also check if libjpeg/jpegtran CLI tool check chroma sampling factor, and it's not the case. So, the libjpeg team said that client applications must do the stuff instead to do it in library, but libjpeg CLI tool do not include at least this kind of check. So do what i said, but not what i do (:-)))... At least my whish to see an UPSTREAM report here is not false to 100%. jpegtran CLI tool must be fixed in this way. Gilles Caulier
In this debian bug, there is a patch against libjpeg to check if transform can be operate safety : https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=189027 And in this file we have some code to get iMCU factor : https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=154845 We need to do something like this in jpegutils.cpp Gilles
All is in this jpegtran.c patch and must be adapted to jpegutils.cpp : https://bugs.debian.org/cgi-bin/bugreport.cgi?filename=204_perfect.dpatch;msg=25;bug=189027;att=1 jtransform_perfect_transform() do all check for rotation and flip operations. Gilles Caulier
Created attachment 93320 [details] patch to process lossy transform if iMUC properties are not compatible with lossless transforms OHHH... wonderfull... jpegtran included in digiKam core HAS this kind of code to check iMCU. There is nothing complex to do... excepted for libjpeg 6.x which do not include this code. Here i have a patch working fine for libjpeg > 6.x. If image iMCU is not compatible with lossless transform, lossy transform is procressed. Gilles Caulier
Bad news : libjpeg 6.x and 7.x miss some property in JPEG codec to check validity of image accordingly with iMCU. The patch proposed before will only work with libjpeg >= 8.0 Gilles Caulier
Git commit ca2f3ae955ebe88fbc4d734f5249e29fbb645269 by Gilles Caulier. Committed on 24/06/2015 at 14:59. Pushed by cgilles into branch 'master'. add perfect JPEG transform checks to prevent broken image rotation/flip when image size is not compatible with IMCU properties. This will only work with libjpeg >= 8.0 BUGS: 320107 FIXED-IN: 4.12.0 M +2 -1 NEWS M +27 -6 libs/jpegutils/jpegutils.cpp M +1 -1 libs/jpegutils/jpegutils.h http://commits.kde.org/digikam/ca2f3ae955ebe88fbc4d734f5249e29fbb645269 diff --git a/NEWS b/NEWS index a825b82..e31b848 100644 --- a/NEWS +++ b/NEWS @@ -30,4 +30,5 @@ BUGFIXES FROM KDE BUGZILLA (https://www.digikam.org/changelog): 023 ==> 345990 - 4.9.0 import from external card reader crashe. 024 ==> 339615 - Crashed after selecting Import (sd card reader). 025 ==> 277242 - MYSQL : using different databases for images metadata and thumbnail cache is broken. -026 ==> +026 ==> 320107 - Flip/Mirror operation doesn't flip/mirror the entirety of the picture (there is a strip left unmirrored) [patch]. +027 ==> diff --git a/libs/jpegutils/jpegutils.cpp b/libs/jpegutils/jpegutils.cpp index 6fd8375..1a150f9 100644 --- a/libs/jpegutils/jpegutils.cpp +++ b/libs/jpegutils/jpegutils.cpp @@ -7,7 +7,7 @@ * Description : perform lossless rotation/flip to JPEG file * * Copyright (C) 2004-2005 by Renchi Raju <renchi dot raju at gmail dot com> - * Copyright (C) 2006-2014 by Gilles Caulier <caulier dot gilles at gmail dot com> + * Copyright (C) 2006-2015 by Gilles Caulier <caulier dot gilles at gmail dot com> * Copyright (C) 2006-2012 by Marcel Wiesweg <marcel dot wiesweg at gmx dot de> * * Parts of the loading code is taken from qjpeghandler.cpp, copyright follows: @@ -329,7 +329,8 @@ bool loadJPEGScaled(QImage& image, const QString& path, int maximumSize) } JpegRotator::JpegRotator(const QString& file) - : m_file(file), m_destFile(file) + : m_file(file), + m_destFile(file) { m_metadata.load(file); m_orientation = m_metadata.getImageOrientation(); @@ -412,9 +413,29 @@ bool JpegRotator::exifTransform(const RotationMatrix& matrix) if (!performJpegTransform(actions[i], src, tempFile)) { - ::unlink(QFile::encodeName(tempFile)); - kError() << "JPEG transform of" << src << "failed"; - return false; + kError() << "JPEG lossless transform failed for" << src; + + // See bug 320107 : if lossless transform cannot be achieve, do lossy transform. + DImg srcImg; + + kError() << "Trying lossy transform for " << src; + + if (!srcImg.load(src)) + { + ::unlink(QFile::encodeName(tempFile)); + return false; + } + + if (actions[i] != KExiv2Iface::RotationMatrix::NoTransformation) + { + srcImg.transform(actions[i]); + } + + if (!srcImg.save(tempFile, DImg::JPEG)) + { + ::unlink(QFile::encodeName(tempFile)); + return false; + } } if (i+1 != actions.size()) @@ -531,7 +552,7 @@ bool JpegRotator::performJpegTransform(TransformAction action, const QString& sr #if (JPEG_LIB_VERSION >= 80) // we need to initialize a few more parameters, see bug 274947 - transformoption.perfect = false; + transformoption.perfect = true; // See bug 320107 : we need perfect transform here. transformoption.crop = false; #endif diff --git a/libs/jpegutils/jpegutils.h b/libs/jpegutils/jpegutils.h index bf2e3de..ef74676 100644 --- a/libs/jpegutils/jpegutils.h +++ b/libs/jpegutils/jpegutils.h @@ -7,7 +7,7 @@ * Description : perform lossless rotation/flip to JPEG file * * Copyright (C) 2004-2005 by Renchi Raju <renchi dot raju at gmail dot com> - * Copyright (C) 2006-2014 by Gilles Caulier <caulier dot gilles at gmail dot com> + * Copyright (C) 2006-2015 by Gilles Caulier <caulier dot gilles at gmail dot com> * Copyright (C) 2006-2012 by Marcel Wiesweg <marcel dot wiesweg at gmx dot de> * * This program is free software; you can redistribute it
Git commit 2e4b33db2329359f196af8161465050184f741f6 by Gilles Caulier. Committed on 24/06/2015 at 16:02. Pushed by cgilles into branch 'frameworks'. bakport commit #ca2f3ae955ebe88fbc4d734f5249e29fbb645269 from git/master to frameworks branch M +30 -5 libs/jpegutils/jpegutils.cpp http://commits.kde.org/digikam/2e4b33db2329359f196af8161465050184f741f6
(In reply to Gilles Caulier from comment #35) > jpegtran included in digiKam core HAS this kind of code to check iMCU. There > is nothing complex to do... Yes, I suppose it is what I see in comment #25 One remark about you patch: it makes a lossy transform if perfect transform cannot be made while the user maybe expected a lossless transform. It would be nice if the user was informed when such lossy transform is made or if the user had the possibility to choose either to crop+lossless transform or to lossy transform. Anyway, thanks for your research and patch creation !
@Gilles: I try with new digikam 4.12: it still doesn't work. Probably because my version of libjpeg is 62 (<80). But how do you manage to have a version > 80 ? Ligjpeg comes from jpegturbo (cfr your comment #26) Latest version is 1.4.1 which contains libjpeg.so.62
Sorry, I reopen, see previous comments: it doesn't work, even with latest libjpeg.
I don't understand well your comment #40. digiKam can deal with libjpeg +6.x, 7.x, 8.x and 9.x. Look into core/libs/jpegutils/ : [gilles@localhost jpegutils]$ pwd /mnt/devel/GIT/5.x/core/libs/jpegutils [gilles@localhost jpegutils]$ tree . ├── CMakeLists.txt ├── iccjpeg.c ├── iccjpeg.h ├── jpegutils.cpp ├── jpegutils.h ├── jpegwin.cpp ├── jpegwin.h ├── libjpeg-62 │ ├── jinclude.h │ ├── jpegint.h │ ├── README │ ├── transupp.c │ └── transupp.h ├── libjpeg-70 │ ├── jinclude.h │ ├── jpegint.h │ ├── README │ ├── transupp.c │ └── transupp.h ├── libjpeg-84 │ ├── jinclude.h │ ├── jpegint.h │ ├── README │ ├── transupp.c │ └── transupp.h └── libjpeg-91 ├── jinclude.h ├── jpegint.h ├── README ├── transupp.c └── transupp.h 4 directories, 27 files For each major libjpeg version, we have backported jpegtrans implementation from libjpeg as well, as this implementation is not shared by the library (this is a main problem from libjpeg). About libjpeg-turbo, the backward compatibility is repsected with offcial libjpeg event if version ID is not the same. In fact libjpeg-turbo version ID will be (must be) detected as the offcial libjpeg version ID. The code to wrap right core jpegtrans implementation with system libjpeg is done with cmake : https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/cmake/modules/MacroJPEG.cmake perhaps something has changed in libjpeg-turbo which is not wrapped properlly with this cmake script... Gilles Caulier
(In reply to Gilles Caulier from comment #42) > I don't understand well your comment #40. I just wanted to say that it doesn't work with latest version of digikam: there is still a strip left unmirrored after a flip/mirror operation. I have an up-to-date Fedora 22 with: - digikam-4.12.0-1.fc22.i686 - libjpeg-turbo-1.4.0-1.fc22.i686 (latest version is 1.4.1, I don't think it would change something) Digikam>Help>Components information says "LigJPEG 62" I was expecting that it would work, but no... Is it normal ?
Did you seen my commit message from comment #37 : "This will only work with libjpeg >= 8.0" So the behavior is normal with libjpeg 6.2. Gilles Caulier
> Did you seen my commit message from comment #37 : Yes I saw you comment: didn't you see my answer in comment #40 ? ;-) I have the latest version of libjpeg-turbo (1.4.0) which has libjpeg 6.2. So how is it possible to make your commit work since it requires at least version 8.0 ??