provide Orientation metadatum in HEIF files

[kfjahnke]

oiio extracts some metadata from HEIF files, but the camera orientation does not seem to be among them. I think the orientation value may be hidden 'somewhere deeper' - exiftool does extract it, together with huge amounts of information - much more than the set of values iinfo produces - like this:

  ...
  | Item 51) 'Exif' (2384 bytes)
  | + [TIFF directory]
  | | ExifByteOrder = MM
  | | + [IFD0 directory with 13 entries]
  | | | 0)  Make = Apple
  | | | 1)  Model = iPhone 12
  | | | 2)  Orientation = 6
  | | | 3)  XResolution = 72 (72/1)
  ...

I tried this with a test image in portrait orientation (exif code 6) from and iphone 12 and iterated over the extra_attribs (this was done in C++ with a v.2.4.7.1 oiio install), and I also tried a build of iinfo from oiio master (oiio 2.6.0.2dev with libheif 1.15.1-1) which did not yield an Orientation value either.

Since this is such a crucial bit of information - especially for a format used on smartphones, which are often used in portrait orientation, gaining access via oiio would be greatly appreciated!

[kfjahnke]

Okay, one step further. In the file src/heif.imageio/heifinput.cpp, the Orientation tag is actively deleted (line 285 ff.):

    // Erase the orientation metadata because libheif appears to be doing
    // the rotation-to-canonical-direction for us.
    m_spec.erase_attribute("Orientation");

When I comment out this line, the Orientation tag becomes available and is correct. I think the notion that libheif automagically does the rotation is erroneous, but this may only be the case for my single test image. I think it's time to ask @lgritz for an opinion on the matter.

[lgritz]

I don't think I would've written it if it weren't true at that time.

Is it possible that libheif USED TO auto-rotate and have the orientation tag reflect the original (pre-rotated) orientation, and then in a later version either stopped doing that or made the auto-rotation some kind of option that defaults to off?

[lgritz]

I don't suppose you could send me (or attach to this ticket) a heif image with non-default orientation so I have a test case?

[lgritz]

I'm pretty sure I have a solution to this, but I need a non-default-oriented HEIF to test on.

[kfjahnke]

Is it possible that libheif USED TO auto-rotate

I can't say; I've only noticed the erroneous behaviour a couple of weeks back.

I need a non-default-oriented HEIF to test on.

I had hopes to find such images in the set of these images here, but they were all in landscape. I asked them to provide some non-standard-oriented ones, but so far noone has answered.
I'll see what I can do my end. Unfortunately, the test image I have shows several people and so I can't pass it on, so I'll have to search for one I can share. I googled for an hour, but it seems such images are really hard to come by - I'll post again when I find one.

[kfjahnke]

It looks like it's getting tricky. I found some samples with non-standard orientation, but they seem to 'automagically' rotate. Maybe my sample is 'shot' somehow. I'l keep on digging and post again.

[kfjahnke]

I've come to conclude that my 'portrait' test image is shot. All the other samples come out right. So I think the code in OIIO's heif plugin is correct after all, and provides correctly oriented output, and width and height metadata referring to the image's extent as it should be viewed - so, with width smaller than height for portrait shots. It does indeed automagically rotate portrait images, and therefore deleting the Orientation metadatum is the sensible thing to do.

Sorry for the noise.

[lgritz]

Aha, we are on the same track. I also found those heic.digital examples last night. I think I have a complete fix coming for you, hopefully later today.

[lgritz]

Near as I can tell, nothing changed on the libheif side. But it does autorotate by default, and we weren't using the right API calls to tell it not to.

The reason we were erasing the Orientation metadata is because of the auto-rotation. The image returned to the caller was already in canonical default orientation, so the metadata reflected that.

I think that by default, we SHOULDN'T autorotate, because we don't for any of the other file formats, and we should leave the Orientation metadata as it was in the file. For compatibility, I will allow an open-with-configuration request that reverts to the auto-rotate behavior (in which case, I will also add a "heif:Orientation" metadata that communicates the original pre-rotated orientation from the file (the usual "Orientation" will continue to describe the orientation of the pixels as presented to the caller).

[kfjahnke]

Thank you for looking into the matter!

I think that by default, we SHOULDN'T autorotate

I think your proposition makes sense, and it's actually what I would have hoped for. I extract the Orientation metadatum if I can get it, set up memory which is 'the right way round' - so, smaller width and larger height for portrait - and use the image reading process with appropriate strides depending on the Orientation tag:

    // xs, ys are the strides of the target array
    // note: vigraimpex uses strides in terms of pixels, not bytes

    int xstride = xs ;
    int ystride = ys ;
    int offset = 0 ;
    switch ( exif_orientation )
    {
      case 0: // no EXIF orientation found
      case 1: // file and memory order coincide
      {
        // we needn't do anything, the strides are set correctly be default.
        break ;
      }
      case 3: // 180 degree rotation
      {
        xstride = -1 ;
        ystride = -ys ;
        offset = ( imageInfo.height() - 1 ) * ys + imageInfo.width() - 1 ;
        break ;
      }
      case 6: // 90 degrees clockwise
      {
        xstride = ys ;
        ystride = -1 ;
        offset = imageInfo.height() - 1 ;
        break ;
      }
      case 8: // 90 degrees counterclockwise
      {
        xstride = -ys ;
        ystride = 1 ;
        offset = ( imageInfo.width() - 1 ) * ys ;
        break ;
      }
      default:
      {
        // the exif tag can also specify flipped images, but this is
        // rare and currently disregarded.
        // TODO: handle 'flipped' orientations as well
        std::cerr << "unhandled EXIF orientation value "
                  << exif_orientation << std::endl ;
        break ;
      }
    }

Running an image viewer with real-time animated sequences, I need the pixels in the right memory order for maximally efficient access - so, smallest stride along the x axis. This works fine for the image types I've been using so far with libvigraimpex, and since I saw that the HEIF files do actually contain an Orientation tag, but OIIO returned zero, I was expecting it to do the same and assumed something was wrong when it didn't. Now you've clarified that the current behaviour is an exception to what OIIO normally does, and this explains why my viewer opened the examples just fine: they were autorotated. My 'portrait' test file, though, seems to have been messed with by feeding it through external software.

So, thumbs up for changing the HEIF plugin to behave like the others. Once the new code comes online, I'll gladly test it.

[kmilos]

One small caveat: the orientation in HEIF files is not (or should not be) stored in Exif - in fact, the Exif value should be ignored if present. The values to use are irot and imir properties. See AVIF example here: https://zpl.fi/exif-orientation-in-different-formats/

[lgritz]

Aha, ok, I will fix. Thanks for the tip.

I think I see how to do it now, there's a whole other heif_properties.h that I wasn't using, and that seems to have functions to decode the transformation commands. It looks a bit tedious, but I think I understand what to do.

[kmilos]

It looks a bit tedious, but I think I understand what to do.

If it helps, in darktable we e.g. read and map the irot + imir combo to existing "Exif" enum.

[lgritz]

Amended

[kfjahnke]

I think your pull request #4142 is stuck with an error in the pipeline. The new code is not yet online.

I think that by default, we SHOULDN'T autorotate

I just hit upon another plugin which seems to autorotate: opening RAW files. I use Canon Cameras producing .CR2 files, and they all come up with an Orientation value of 1 and width and height set to the extent of the image after rotating it, so small width and large height for portrait formats.

[kfjahnke]

OIIO can avoid the autorotation with

config [ "raw:user_flip" ] = 0 ;

But then one needs the orientation so one can flip the data after receiving it (or pass appropriate strides). The orientation can be obtained by querying raw:flip - I think this value should turn up as the Orientation value and the file should be opened with raw:user_flip=0. Then you'd be consistent with the 'no autorotation' paradigm. Here's a snippet from the libraw docu:

Structure libraw_output_params_t: management of dcraw-style postprocessing

Structure libraw_output_params_t (imgdata.params) is used for management of dcraw-compatible calls dcraw_process(), dcraw_ppm_tiff_writer(), and dcraw_thumb_writer(). Fields of this structure correspond to command line keys of dcraw.
Data fields:
[snip]
int user_flip;
    dcraw keys: -t
    [0-7] Flip image (0=none, 3=180, 5=90CCW, 6=90CW). Default -1, which means taking the corresponding value from RAW. 

In the raw input plugin, you code

    // Force flip value if needed. If user_flip is -1, libraw ignores it
    m_processor->imgdata.params.user_flip
        = config.get_int_attribute("raw:user_flip", -1);

which extracts raw:user_flip, not raw:flip, so this can't work. I think the solution would be to transfer raw:flip to Orientation and pass 0 to user_flip unconditionally to avoid any rotation, as stated above.

[kfjahnke]

Just to add to the confusion: dcraw flip key 5 translates to exif orientation 8. And 0 to 1. argh...

[lgritz]

@kfjahnke Does the change I propose to heif reading look ok to you at this point? If so, can you please comment on the PR?

I can handle the raw issue separately. Or would you prefer that they get wrapped up in the same PR because the point is to make their behavior consistent?

[kfjahnke]

Sorry, somehow your comment did not make it to my email account and I've only seen it now. I'll look at the PR soon - I did not look at it yet because it did not pass all tests in your CI/CD pipeline - I'm not sure if you noticed. So I'll have to inspect the 'raw' PR rather than pulling from master, because the changed code isn't yet online. Sorry for the delay.
I think handling the raw issue with a second PR is better. Should I raise another issue?

[lgritz]

The CI failures are for reasons completely unrelated to that PR. In this case, it's one of the CI tests that tries to build against the current top-of-development tree of several important dependencies, which of course sometimes break, and that's expected.

[lgritz]

Yes, if you don't mind, please open a second issue about the raw stuff and we can handle that separately there, after merging the heif part (if you think what I've done there seems ok for now).

Though the raw+heif issues together raise in interesting question about what our policy should be for rotated images.

For formats that we read with 3rd party libraries that DON'T have the option of rotating for us (like libtiff for TIFF files), we have always just returned the image at it is in the file, plus the metadata that says in what orientation it should be displayed.

But what about formats where we use 3rd party libraries that DO have the ability to rotate the image into display orientation for us? What should we do?

• Should we also read them in their original layout, just as we do with libtiff?
• Should we auto-rotate if the format-reading library has the ability to do it as it reads it for us? (That's what we previously did for libheif, but now we're changing it.)
• Should we have a configuration hint that lets the caller choose whether to rotate or not (that's what we're changing the heic reader to do)? And if so, should the default be to rotate or not?

I think if we can articulate which of these choices is our preferred behavior to apply uniformly, we'll be more confident about exactly how to change heif and raw readers and make sure to handle any other formats in a consistent manner.

[kfjahnke]

The CI failures are for reasons completely unrelated to that PR. In this case, it's one of the CI tests that tries to build against the current top-of-development tree of several important dependencies, which of course sometimes break, and that's expected.

Ah, I thought that kept your PR from being available from OIIO master. Maybe I was just imaptient and it hadn' trickled through when I tried.

please open a second issue about the raw stuff

will do.

if you think what I've done there seems ok for now

I just commented on your PR #4142, please have a look. I did not actually test the code yet, because I think it may have issues.

But what about formats where we use 3rd party libraries that DO have the ability to rotate the image into display orientation for us? What should we do?

If autorotation is available, it's a fine thing to have, because it presents the images preconditioned so that they can be displayed without any trickery. To a program reading images with OIIO, it should be irrelevant whether an incoming image in portrait format resulted from an image file where the pixels were already stored in that memory order or whether they were converted to this format by autorotation - in both cases, the 'Orientation' value should be 1. But it should be possible to switch off the autorotation, as can be done e.g. for RAW images by passing raw:user_flip = 0. If autorotation is off or the plugin does not offer autorotation, the 'Orientation' value must be available, because then the user has no other way of getting the image to display right.

Why switch off autorotation? Here's an example from my work: When setting up source images of a panorama for registration, once the image properties have been gleaned and control points are extracted, there is process which tries to adjust all parameters to an optimal registration, among them the field of view. Most panorama software uses the 'horizontal field of view' - but what is horizontal? If you refer to the sensor, it's always along the longer central axis, but if you refer to autorotated images it might be along the shorter central axis. If you have a mixed set and refer to autorotated images, you have two separate hfov values to adjust, because the software doesn't necessarily 'see' how they are related. So to make it easier for the 'Optimizer', it's best to feed sensor data without any meddling - so, no autorotation. Then, the optimization process will produce appropriate camera roll values, while the images all have a common aspect ratio and equal fov. On the other hand, if a user has set up a panorama with autorotated images (e.g. made with dcraw and autorotation) and you want to 'slot in' the RAW instead, keeping the registration data unchanged, you must emulate dcraw's behaviour and autorotate the images on import. If you're interested, I have coded this handling of RAW data in lux (the oiio branch has it) because I have both kinds of panoramas, and I can only slot in the RAWs for the TIFFs if I deal with this issue correctly. The oiio branch also has a facility to pass extra variables through to the plugins, which makes it a good tool for experimenting with these values - and for the case just described, I can pass it the raw:user_flip value on the command line.

So, to wrap up, here's what I think OIIO should do:

• if autorotation is available, use it per default and yield 'Orientation' = 1, but
• allow the user to switch autorotation off (no effect if it can't be had in the first place)
• if no autorotation was performed, provide an EXIF-compatible 'Orientation' value

[lgritz]

So, to wrap up, here's what I think OIIO should do:

• if autorotation is available, use it per default and yield 'Orientation' = 1, but
• allow the user to switch autorotation off (no effect if it can't be had in the first place)
• if no autorotation was performed, provide an EXIF-compatible 'Orientation' value

Thank you, this is precisely the kind of guidance I was hoping for. I think this is a very reasonable policy and well articulated.

[kfjahnke]

Glad you appreciate it. Really, autorotation as default is matter of taste. You seemed quite firm in wanting it disabled by default, but I think that keeping it on by default is the better option. It shouldn't break old code, but allowing to disable it leaves it up to the user to get hold of the camera orientation and handle it 'manually' if desired. Just a slight modification to the policy: the last point should really be:

• if no rotation was applied, provide an EXIF-compatible 'Orientation' value

This is to clarify what happens when the user passes in arguments to deliberately rotate the image in a particular way: then one might assume they know what they are doing and present the outcome as 'correctly oriented' by setting Orientation to 1. I think this is not an issue with HEIF, but for RAW files with the raw:user_flip parameter it is. It does require looking into the parameters, though, and you may not like that.

[lgritz]

Really, autorotation as default is matter of taste. You seemed quite firm in wanting it disabled by default, but I think that keeping it on by default is the better option.

You have me convinced. I would say that my previous opinion was simply that since some of the earliest input readers we had (like tiff and jpeg) did not auto-rotate -- because their underlying libraries did not offer a way to do it without extra trouble and cost -- I just assumed this should be the default behavior and other plugins should confirm. I obviously broke my own rule with libheif, so now we need to reshuffle the "policy" to describe what we think is best, and I agree that the behaviors you suggest are totally reasonable.

I resubmitted the PR, I think incorporating all the aspects of our new shared agreement about desired behavior of the heif reader.

[kfjahnke]

Great! I saw this quote attributed to Lars Wirzenius which I requote here:

Policy is your friend. Trust the Policy. Love the Policy. Obey the Policy.

So, OIIO,

reshuffle the "policy"

I added a comment to your resubmitted PR #4142 re. uniform use of EXIF Orientation semantics which you may want to consider as for inclusion in the policy as well - it's just about the policy, your libheif plugin code already uses EXIF values.

I think the new policy is reasonably similar to the previous behaviour, which is an advantage, because it shouldn't break anyone's code unless they've dug really deep and interacted with the plugin directly. Having a policy definitely beats making it up as you go along ;-)

[kfjahnke]

I looked at your new code in heifinput.cc:

            // libheif supplies oriented width & height, so if we are NOT
            // auto-reorienting and it's one of the orientations that swaps
            // width and height, we need to do that swap ourselves.
            if (orientation == 5 || orientation == 6 || orientation == 8) {
                std::swap(m_spec.width, m_spec.height);
                std::swap(m_spec.full_width, m_spec.full_height);

I think Orientation 7 is also portrait. I usually code if ( orientation > 4 ) to catch portrait orientation.

[lgritz]

I think Orientation 7 is also portrait. I usually code if ( orientation > 4 ) to catch portrait orientation.

You are correct, I will fix, thanks.

[lgritz]

Updated the PR with that last fix, thank you. I really appreciate the close scrutiny of the details here.

[kfjahnke]

I really appreciate the close scrutiny of the details here.

You're welcome. That wasn't so hard to spot... and I want the code to work for me as well.

I would say that my previous opinion was simply that since some of the earliest input readers we had (like tiff and jpeg) did not auto-rotate -- because their underlying libraries did not offer a way to do it without extra trouble and cost -- I just assumed this should be the default behavior and other plugins should confirm.

Having thought about autorotation some more, I think that adding it as a feature to libraries which don't provide it might be quite simple. Further up, I've already shared a snippet of code which I use to handle incoming data in my program. The key is to manipulate the origin and strides into the target memory buffer. I revisited the code recently to add handling of flipped orientations, which turned out to take just another dozen or so lines of code. Here's how it's done, in a nutshell:

• the receiving side needs a data buffer already shaped to either landscape or portrait to accept the data.
• take origin and strides of that buffer (or receive them as arguments from the user if they aren't automatic)
• then, depending on the Orientation value, adapt the origin and strides
• now read the data with the adapted origin and strides
• voilà, the target buffer contains prerotated data and can be used e.g for display as-is

I presume that you use stride-aware code at the bottom and the unstrided access maps internally to using (default) strides, so it's just a matter of slotting in the modified origin and stride at the right moment. I won't clutter this space with yet more code (unless you ask me to) - the code lux uses is here - line 1841ff as of this writing. The download section has an up-to-date AppImage if you want to see the autorotation in action.

Similar code in OIIO might provide autorotation as a general feature, even for those libraries which don't natively offer it. I think the amount of code needed to add this as a feature would be quite manageable, and the advantage would be that users could stop worrying about getting the rotation right unless they need to handle it 'manually'.

[lgritz]

The primary "image-oriented" interface OIIO provides for applications that deal with whole images and don't want to know the gory details is ImageBuf, and OIIO already offers the ImageBufAlgo::reorient() function, which will transform a full ImageBuf according to its "Orientation" metadata so that the pixels are in the Orientation=1 display-preferred layout. So it's already easy to do for applications using OIIO without needing to implement it manualy.

But ImageInput is intended to be a lower-level, file-oriented interface whose fundamental operation is to read one or more scanlines or tiles (specified by its index range) into a buffer sized and shaped for that number of scanlines or tiles. There is a readimage() method, but it's just a small convenience function that wraps either read_scanlines or read_tiles (depending on which the file is).

So, you see, if somebody wants to read scanline 47... I think they actually want scanline number 47 from the file. It's not at all clear to me that we should be trying to play games with the strides behind the caller's back to give them a different part of the image than they asked for, just because the camera was held sideways when the image was made. The image is the image, and I think that's what ImageInput should return.

That's why I've always been a little leery about any auto-rotation at all at the ImageInput level, especially if it was automatic. But the exception I made is that if we are relying on a 3rd party library to read the file format, and it is already doing the work of rotating the image as it presents it to our calls then maybe it's ok for us to just let it do that, especially if the library is already fundamentally reading the whole image into a buffer, so there is not really any advantage to doing scanline or tile operations individually. The thing we're changing in this PR is to modify this exception slightly: (a) saying that if the underlying library supports it and there's no particular disadvantage in auto-rotating, it's fine for that behavior to be the default; and (b) we're picking a single standard convention for how to opt out of that behavior, and how to indicate in the metadata what has happened.

[kfjahnke]

Thanks for clarifying. That makes sense. I really want the low-level access. I am doing the 'stride magic' on import because I want to keep the time to 'first light' as short as possible - running an image viewer, and especially a panorama viewer, where the files can be quite large, you want to keep the user occupied rather than showing them a blank screen or an hourglass or a beach ball. So I try and grab the data as fast as I possibly can, calculate a view with the best interpolation method I can offer without seriously processing the data and show that view. Now the user has something to distract them from the work going on in the background, where the rendering code builds image pyramids for multiscale viewing and prefilters the b-splines for interpolation. If you try out lux, you may appreciate how soon you get 'first light'.

already offers the ImageBufAlgo::reorient() function, which will transform a full ImageBuf

The down side of this approach is that you have to have a full ImageBuf in the first place: you can only have it if you read the entire image, and then you do something at this level. If you have it on the GPU side, you're probably all right, but my program is CPU-based and uses multithreaded SIMD code for speed. If I can avoid code which reads an image from one buffer and writes it back or to another buffer, I do. What I'm aiming at is not to use an import into an image buffer initially at all, and that's one of the reasons why I'm so interested in OIIO: OIIO can provide access at the scanline/tile level, and I can use my 'wielding' code to suck scanlines/tiles into a SIMD pipeline which can do stuff like range mapping, type conversion, colour space manipulation etc. and, finally, deposit the result in an image buffer in a form where it does not need further processing, but is ready to be used by the view-generation code - in contrast to my current approach, where I suck in the raw data and stick these processing steps into the pipeline creating the view from the 'raw' image data.
I developed the 'wielding' and SIMD code initially as part of my library vspline, and recently I have factored it out to a new library, zimt. I have established that zimt can interface easily with OIIO's scanline/tile code (zimt even has tiled data set handling code), so I think I'll make an attempt to drop image reading to the scanline/tile level and throw a bit of processing in at that point. The image data coming in flipped, rotated or 'straight from the sensor' is all fine with me, as long as I can get the right metadata - if the data aren't preoriented, I can use the 'stride magic' my end - and I can pass the strides to OIIO, which is great.
I have always wanted to get 'a foot in the door' at that point of the process. Even though working at the scanline/tile level needs a bit of memory, it's so little that it's likely all in cache, and since disk I/O is so slow, even being limited to a single thread is not a grave problem, because with this scheme, the data are already all preprocessed by the time the last scanline is consumed. I'm sure you can see the beauty of this approach: rather than waiting around until the entire image has made it to a buffer and then work at the entire-image level, you do the work while the image loads, and when the load is done, the data are already conditioned as you need them.

[lgritz]

IBA::reorient is for applications that want the simplicity of a single call and are fine with the expense of an extra buffer copy as well as not needing to do the I/O on an individual scanline or tile basis.

For applications such as yours, do you know that the existing ImageInput read_scanline(s) and read_tile(s) already take optional byte strides? So for an application that wants this, it's already possible to read the scanlines and tiles into the rotated positions.

I guess the policy questions to grapple with are: (a) Should ALL file format readers, if their format supports something akin to display orientation, be required to support reorientation in the ImageInput, or disallowed from doing it, or leave it as a per-format choice? (b) If they do support reorientation, should they do it by default (i.e., is it opt-in or opt-out)?

There are many possible answers, and to be honest, I'm still not very confident which is best. One alternative is to make auto-reorientation both mandatory and default for all formats, so they behave 100% consistently. Another is to never auto-reorient in the ImageInput, and such things should be entirely up to the app to pass the right strides or handle it in whatever way they choose. There are also policies somewhere in the middle.

Before this PR, the de facto policy was in general not to auto-rotate, but we allowed exceptions for raw and heif because the underlying libraries leaned heavily to doing it automatically for us and made it more convenient to let them do that than to try to stop them. (In fact, prior to this PR, we didn't have a way to tell libheif NOT to.) With this PR, we're standardizing hint convention to opt out of reorientation (for those readers that do it), and also saying explicitly that it's ok for a reader to have the default of auto-rotating (before, we said they shouldn't, and yet we let a couple of them do it). So I guess primarily we're simply blessing the behaviors we were doing in practice that were against policy, but not forcing any other readers to change their behaviors.

I'm pretty confident that a bad choice would be to force all formats to reorient by default. That would certainly break lots of apps that expect the current behavior from their favorite formats. I don't mind heif and raw reorienting by default, since that's what they've always done, so this is keeping existing behavior stable, though at the expense of having a single consistent behavior for all formats. It's always a tradeoff.

[kfjahnke]

For applications such as yours, do you know that the existing ImageInput read_scanline(s) and read_tile(s) already take optional byte strides? So for an application that wants this, it's already possible to read the scanlines and tiles into the rotated positions.

So far I use the strides arguments for entire images only, but I may use them for tiles in the future to 'frame' image data with a bit of support which is needed to provide decent interpolation even for pixels which are located on the edge of tiles. But I'm aware of the possibility, this is what made me start using OIIO in the first place!

So I guess primarily we're simply blessing the behaviors we were doing in practice that were against policy, but not forcing any other readers to change their behaviors.

Yes, now there's a consistent policy, but the behaviour is unchanged. This is the best for users, because it won't break their code. But at the same time we have a clear guidance on how to proceed in the future when new plugins come in.

I'm pretty confident that a bad choice would be to force all formats to reorient by default. That would certainly break lots of apps that expect the current behavior from their favorite formats. I don't mind heif and raw reorienting by default, since that's what they've always done, so this is keeping existing behavior stable, though at the expense of having a single consistent behavior for all formats. It's always a tradeoff.

If you put it like that I'd agree. But I was thinking more of a global OIIO configuration argument which would prescribe that if incoming data do in fact yield an orientation which is not 0 or 1 'stride magic' should be applied to pre-orient them during the read. So this would be an additional opt-in request by the caller to handle the rotation/flip for them, even for those formats which don't provide it natively. What I hoped to demonstrate by the code I linked to is that this can be done entirely by manipulating stride and origin of the target buffer, so it might be slotted in at a high level (the plugin would not even be aware of it). Again, in a nutshell:

• if the user actively requests pre-orientation
• apply stride magic to the incoming target's origin pointer and strides (if Orientation is 0 or 1, 'stride magic' has no effect)
• then pass the - potentially modified - origin and strides arguments to the plugin

To the plugin it'd look as if it had been called with the given strides by the user, while, in fact, the library has interceded and manipulated the origin and strides to affect pre-rotation. It'd be up to the user requesting such behaviour to pass in a buffer with the right structure - e.g. when the data are EXIF portrait, their buffer must be width < height. Since it'd be an opt-in, you could prescribe that - users using opt-ins need to be aware of the requirements to use the opt-in successfully. I think that the coding effort would be small - if the opt-in is present, call the routine to do the 'stride magic' and pass on the modified arguments.

Now you may say 'why bother', and my answer would be 'speed and convenience'. Of course users can already look at the Orientation, and if it's non-standard, do the 'stride magic' themselves and pass in appropriate strides and origin. In fact, that's precisely what I do - successfully, as demonstrated by the experimental lux AppImage I provided. But the code isn't quite trivial, and on the other hand, if it's in the library it's good for everything and for all users and would help them to get on with their actual image processing without having to twiddle bits - and without first having to read the entire buffer and then do something with it at the entire-image level. It's just what you write in your docu - clever things can be done with the strides. You explicitly give flipping the image as an example, so why not go beyond mere suggestions and provide a little useful stride magic by the library? Again, just my fifty cents.

To wrap up:

• what we have agreed upon so far is a way to to optionally suppress autorotation in plugins which offer it
• what I propose is a global config argument to ask for pre-rotation of all data which the plugin marks as having non-standard orientation

And the two would play together: a plugin which is asked not to autorotate would yield a non-standard Orientation, and if global pre-rotation is on, the data would then be pre-rotated by 'stride magic' - the outcome should be the same as using autorotation. Whether this behaviour could/should be applied to scanline and tile access would need to be discussed - I think these access modes are low-level and should work on the data as stored in the source data, so for these I'd even force the flag to cancel autorotation and ignore the flag to pre-rotate.

[lgritz]

But I was thinking more of a global OIIO configuration argument which would prescribe that if incoming data do in fact yield an orientation which is not 0 or 1 'stride magic' should be applied to pre-orient them during the read.

I do not oppose using the non-default hint "oiio:reorient" = 1 on the open call being a request to re-orient if the reader supports it, and I do not oppose any PR submissions that add this ability to any existing readers (as long as the implementation doesn't clutter them up too badly -- for heif and raw, it was little more than a single parameter to the underlying library).

I'm currently opposed to setting policy that all readers should support this hint, simply because it's a hollow policy unless we have somebody stepping forward to actually do the work. I'd rather accept the PRs adding the optional hint, which can happen over time, and if and when we get full coverage over all the readers, then we can change the policy to say it's required including for any new formats we add. In other words, the rules need to match what we're capable of doing on a reasonable timescale.

I'm also somewhat opposed to a global switch, because this is a library that may be used by different pieces of separately developed code within one application's execution space (for example, a big DCC app and three different plugins developed by different companies may all be using a single instance of OIIO. I know that we do have plenty of other global options, but none that do anything as drastic as rearranging which pixels show up where in an image. It seems safer to make this a per-open option, so that different sections of code sharing OIIO can't clobber each other as easily.

[kfjahnke]

Okay, I see your point. I have thought about how what I had in mind can be achieved in a less obtrusive fashion and without having to meddle with all plugins, and I think I've found a simple answer: make it a utility function which the user can call to calculate adapted strides and offset for them. I have a rough first version like this, just so you get the gist of it:

void gyrate ( int orientation , long width , long height ,
              long & xstride , long & ystride , long & offset )
{
  // copy of the strides

  long xs = xstride ;
  long ys = ystride ;
  bool flipped = false ;
  offset = 0 ;

  switch ( orientation )
  {
    case 2:
    {
      flipped = true ;
      // fallthrough is deliberate
    }
    case 0: // no EXIF orientation found
    case 1: // file and memory order coincide
    {
      // we needn't do anything, the strides are set correctly be default.
      break ;
    }
    case 4:
    {
      flipped = true ;
      // fallthrough is deliberate
    }
    case 3: // 180 degree rotation
    {
      xstride = -xs ;
      ystride = -ys ;
      offset = ( height - 1 ) * ys + ( width - 1 ) * xs ;
      break ;
    }
    case 7:
    {
      flipped = true ;
      // fallthrough is deliberate
    }
    case 6: // 90 degrees clockwise
    {
      xstride = ys ;
      ystride = -xs ;
      offset = ( width - 1 ) * xs ;
      break ;
    }
    case 5:
    {
      flipped = true ;
      // fallthrough is deliberate
    }
    case 8: // 90 degrees counterclockwise
    {
      xstride = -ys ;
      ystride = xs ;
      offset = ( height - 1 ) * ys ;
      break ;
    }
    default:
    {
      std::cerr << "unhandled EXIF orientation value "
                << orientation << std::endl ;
      break ;
    }
  }

  if ( flipped )
  {
    if ( orientation < 5 )
    {
      offset += xstride * ( width - 1 ) ;
    }
    else
    {
      offset += xstride * ( height - 1 ) ;
    }
    xstride = - xstride ;
  }
}

Maybe that could go into the "Public Static Functions" section, where there's other stuff dealing with strides. What do you say?

[lgritz]

Yes, I love that idea! It would be very helpful to provide a single function that computes the strides you need to pass to get_{scanline,tile,image} to shuffle each read from the native orientation to the preferred display orientation. That's not something each application should have to figure out on its own (and probably make mistakes).

[kfjahnke]

Since this would actually mean that my code makes it into OIIO, there's licensing issues to clarify. At this point, I don't wish to sign the CLA and I won't reformat my code to other standards than my own. The code is currently part of the lux code base and therefore licensed under GPL, but you may not be able to use GPL-licensed code in OIIO. To make it easy for you to pull in the code, I'd consider publishing the relevant function as an MIT-licensed github gist, which should give you all the rights you need. Would that be acceptable? You can have a look at the code here (line 64ff) - the function is named 'gyrate' - it's roughly what I suggested above, with a bit of tweaking.

[lgritz]

We can't use GPL-licensed code, but if you wrote the code originally, having submitted it to one project under GPL doesn't prevent you from submitting it to another project under a different license.

We do require a CLA to submit code to OIIO. So if you're not willing to make a PR yourself, then someone else would need to step forward to write new code or copy existing from someplace with a compatible license (MIT, BSD, Apache).

[kfjahnke]

someone else would need to step forward to [snip] copy existing from someplace with a compatible license (MIT ...

Would you? I can make the code available under MIT license, e.g. as proposed above as a github gist, or alternatively as utility code in one of my MIT-licensed projects - zimt already has some examples using OIIO, so would fit in there.

[lgritz]

Would you?

This isn't easy to say and it may be hard to understand. But it's a strategic priority for this project to wean itself off so much being dependent on me. I'm trying to make a concerted effort to NOT jump in and do things that could be easily done by others. Perhaps I should not even have done this recent HEIF patch, but I bent the rules a bit because it's related to policy, which is my role to do.

As we like to say here: 🎣 Mackerel!

[kfjahnke]

I wish you best of luck with the weaning-off process, but I have my doubts whether the project will simply float without you jumping in just about everywhere. It looks to me like 'in the old days' you could get somewhere writing innovative, relevant software - people would find out about it, become interested, start interacting, contributing, and a community would form. Nowadays attention spans are counted in seconds and search engines provide data which are likely to increase the mother company's profits - the internet does not value relevance or originality above popularity. So if you want your project to get ahead, you have to be the person-of-all-trades, doing everything from genuine programming to bug fixing to documentation to legal stuff to packaging. If you want it to become popular as well, you may need to hire an influencer and pay for search engine optimization on top.

I'll stop myself from ranting now. Let's leave it at that. If any of you people in OIIO feel like pulling in my 'gyrate' code, I can double-license it to MIT, just ask. Otherwise people can just write their own version. This isn't rocket science, just a handy little utility routine. Sorry for my aversion to CLAs - it's my opinion that as a software author I've done my bit when I publish my stuff and offer it under a given license. Separate agreements with specific entities under their terms would require me to fully look through their legalese.

[lgritz]

I totally get where you're coming from on the CLA. You're right, it should be enough to offer code to the project. The CLA is really in place to protect the authors from claims by their employers or others, and to protect the downstream recipients of the software from the risk of having critical features removed if there was some kind of dispute over authorship. It's admittedly a big pain in the ass, but it protects the authors from a variety of shenanigans that some of their employing companies have been known to pull in the past. That's the rationale, anyway. We aren't just trying to make thing hard for you.

If you aren't able to directly contribute this feature, I'm hoping that somebody else reading this can. I don't think it's really dependent on your existing code -- frankly, it's short enough that it's probably not a significant savings over writing the equivalent from scratch and avoiding the whole licensing issue.

[kfjahnke]

frankly, it's short enough that it's probably not a significant savings over writing the equivalent from scratch and avoiding the whole licensing issue

Took me a while to get right, though ;-)

I think there's a grey area about how original a work must be to be protected by copyright, and this function probably does not qualify. But that's up to the person grabbing the code to decide - will they risk being sued for copyright violation? Hence the license. And I'm sure you're not trying to make it hard for me. I trust we're all the 'good guys' here.