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 770917 - [PATCH] Raw Rotated images are truncated
[PATCH] Raw Rotated images are truncated
Status: RESOLVED OBSOLETE
Product: GEGL
Classification: Other
Component: operations
git master
Other All
: Normal normal
: ---
Assigned To: Default Gegl Component Owner
Default Gegl Component Owner
Depends on:
Blocks:
 
 
Reported: 2016-09-05 22:44 UTC by Jean-Baptiste Mayer
Modified: 2018-05-22 12:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (1.26 KB, patch)
2016-09-05 22:44 UTC, Jean-Baptiste Mayer
committed Details | Review
Revert "Raw load (libraw): bounding box correction for rotated images" (2.02 KB, patch)
2017-12-06 14:16 UTC, Debarshi Ray
committed Details | Review
operations/external/raw-load: Optionally use LibRaw to orient the image (2.32 KB, patch)
2017-12-07 09:53 UTC, Debarshi Ray
none Details | Review

Description Jean-Baptiste Mayer 2016-09-05 22:44:38 UTC
Created attachment 334859 [details] [review]
patch

Hi,

While processing CR2 (Canon RAW) and NEF (Nikon Raw) images I found that my horizontal pictures would process fine but vertical would crop to square and rescale non proportionally.

For these vertical pictures get_bounding_box would return unrotated picture dimensions while process() would fill a buffer of rotated dimensions. This seems to be an old issue - see https://bugzilla.gnome.org/show_bug.cgi?id=755078#c1 . Funnily enough this was also the case with the old DCRaw-based loader.

This patch swaps the width and height of the picture obtained from libraw when libraw indicates the image is to be rotated +/-90 degrees.

This patch was tested against an admittedly unrepresentative sample of holiday photos - both vertical and horizontal.
Comment 1 Øyvind Kolås (pippin) 2017-04-27 15:07:45 UTC
commit 14d71e541270d0248f4307d3701c26759673205b
Author: Jean-Baptiste Mayer <jean-baptiste.mayer@m4x.org>
Date:   Mon Sep 5 18:47:10 2016 +0100

    Raw load (libraw): bounding box correction for rotated images
    
    Fixed bug: 770917
Comment 2 Massimo 2017-10-22 12:32:44 UTC
(In reply to Øyvind Kolås (pippin) from comment #1)
> commit 14d71e541270d0248f4307d3701c26759673205b
> Author: Jean-Baptiste Mayer <jean-baptiste.mayer@m4x.org>
> Date:   Mon Sep 5 18:47:10 2016 +0100
> 
>     Raw load (libraw): bounding box correction for rotated images
>     
>     Fixed bug: 770917


This patch was developed before commit:

https://git.gnome.org/browse/gegl/commit/operations/external/raw-load.c?id=2ca38acae33fe9db4883849bf194157de2d5594f

relevant part:

+ p->LibRaw->params.user_flip = 0;


but was applied after it 

https://git.gnome.org/browse/gegl/log/operations/external/raw-load.c

and, unless I'm missing something, now vertical raw images are cropped again.
Comment 3 Massimo 2017-10-23 09:37:23 UTC
BTW, gegl:raw-load calls 'gegl_buffer_set' with the whole image rectangle
and the same data as many times as many chunks are used to complete the
graph processing.

For instance:

gegl -x "<gegl><gegl:raw-load path=\"nikon_d5100_05.nef\" /></gegl>" -o tmp.ppm

calls it 32 times.

image from

http://www.photographyblog.com/reviews/nikon_d5100_review/sample_images/
Comment 4 Debarshi Ray 2017-12-05 20:09:33 UTC
(In reply to Massimo from comment #2)
> This patch was developed before commit:
> 
> https://git.gnome.org/browse/gegl/commit/operations/external/raw-load.
> c?id=2ca38acae33fe9db4883849bf194157de2d5594f
> 
> relevant part:
> 
> + p->LibRaw->params.user_flip = 0;
> 
> 
> but was applied after it 
> 
> https://git.gnome.org/browse/gegl/log/operations/external/raw-load.c
> 
> and, unless I'm missing something, now vertical raw images are cropped again.

Good catch!

I have been wondering lately why oriented RAW images are behaving differently compared to JPEGs and PNGs, and this is probably the reason. I'd argue that this bug is a WONTFIX and that commit 14d71e541270d024 should be reverted.

The JPEG and PNG decoders don't apply the EXIF orientation field. The inconsistency of having the RAW loader applying the orientation makes things needlessly complicated for applications.

I have been discussing this issue of applying the EXIF orientation with mitch and pippin for a while. I think that we anyway need a better codec API for GeglBuffer because the current graph-based approach with source operations has various problems [*]. To ensure more widespread adoption of GeglBuffer, replacing GdkPixbuf, I think we need something that resembles GdkPixbuf's application facing API.

[*] The biggest issues are: they are not asynchronous, don't use GCancellable and no error handling.
Comment 5 Debarshi Ray 2017-12-06 14:15:17 UTC
(In reply to Debarshi Ray from comment #4)
> I have been wondering lately why oriented RAW images are behaving
> differently compared to JPEGs and PNGs, and this is probably the reason. I'd
> argue that this bug is a WONTFIX and that commit 14d71e541270d024 should be
> reverted.

Or, we can add a boolean property to gegl:raw-load that enables application of the embedded orientation, but we should take care that gegl:load continues to be consistent across all image formats.
Comment 6 Debarshi Ray 2017-12-06 14:16:30 UTC
Created attachment 365116 [details] [review]
Revert "Raw load (libraw): bounding box correction for rotated images"

Untested because I don't have access to oriented RAW files at the moment. Will try it out later today.
Comment 7 Øyvind Kolås (pippin) 2017-12-06 14:21:01 UTC
Comment on attachment 365116 [details] [review]
Revert "Raw load (libraw): bounding box correction for rotated images"

perhaps the best way to get this patch tested is pushing it to master - and set the bug to needinfo until someone with appropriate testing files test it?
Comment 8 Debarshi Ray 2017-12-06 14:32:18 UTC
Ok, pushed. I have lots of affected files on my other laptop, but I will only have access to it after work. So, I will test it later today.
Comment 9 Debarshi Ray 2017-12-06 14:33:12 UTC
@pippin: Do you think it makes sense to have the auto-application of orientation behind a boolean property on gegl:raw-load?
Comment 10 Debarshi Ray 2017-12-07 09:52:37 UTC
Removing NEEDINFO after testing. Commit d3e79f4a5060bac7 works for me.
Comment 11 Debarshi Ray 2017-12-07 09:53:16 UTC
Created attachment 365181 [details] [review]
operations/external/raw-load: Optionally use LibRaw to orient the image
Comment 12 GNOME Infrastructure Team 2018-05-22 12:14:27 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gegl/issues/37.