Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fujifilm_dynamic_range only sets bias for first image when importing duplicates #390

Open
dtorop opened this issue Apr 6, 2022 · 10 comments

Comments

@dtorop
Copy link
Contributor

dtorop commented Apr 6, 2022

When an image is imported with extant duplicates (multiple XMP files per raw file) and fujifilm_dynamic_range is enabled, the raw exposure bias will be set for only the first of the duplicates.

Test case:

  1. Place in a directory a raw file which already has an XMP file from a previous import, e.g. _DSF0001.RAF and _DSF0001.RAF.xmp.
  2. Make a duplicate of the XMP, e.g. _DSF0001_01.RAF.xmp
  3. Start darktable with a clean database
  4. In the script manager, start script fujifilm_dynamic_range
  5. Import the raw file -- this will result in two images in the film roll, each a duplicate of the single raw file
  6. Examine the exposure bias of each image (either via image information panel, or in the exposure module in darkroom view)

The exposure bias of the two duplicates won't match. The first will be non-zero (-0.7, -1.7, or -2.7). The second will show a exposure bias of +0.0.

In the usual case of importing un-edited photographs and making duplicates, everything is fine -- the exposure bias is copied with the duplicate. But things break when importing a directory with extant sidecar files with duplicates. In this case, the exposure bias is lost for all but the first image.

The problem is that fujfilm_dynamic_range catches the post-import-image signal on RAF file import, and then only sets the exposure bias for the first of the duplicates.

I haven't looked into a solution yet (is there an obvious way to traverse duplicates via the lua API?), but wanted to document this.

An underlying problem may be that exposure bias is stored in the database, but not in sidecar files. If it were stored in the sidecar, then perhaps fujifilm_dyamic_range could be coded to not set the exposure bias if an image has the is_altered flag set to true.

@dtorop
Copy link
Contributor Author

dtorop commented Apr 7, 2022

Here are better steps to reproduce, without requiring any filesystem-level work. This is to show that the current code can result in inconsistent state:

  1. Start darktable with a clean database.
  2. In the script manager, start script fujifilm_dynamic_range.
  3. Via import -> add to library..., import an RAF (Fujifilm raw) file, e.g. this sample
  4. The image will appear in dakroom view. Switch back to lighttable view.
  5. Make sure the imported image is selected. Click selected image[s] -> duplicate.
  6. Examine the resulting two images in image information. Note that both have the same (non-zero) exposure bias. (In the example image above it is -0.72 EV.)
  7. Select both images (original & duplicate). Click selected image[s] -> remove and confirm the warning dialog.
  8. Go back to import -> add to library... and re-import the image. The original will appear in darkroom view.
  9. Examine the two imported images via image information. Note that the second has a zero (+0.00 EV) exposure bias. Note also that (in a standard setup) the second image will appear darker than the original.

@phweyland
Copy link

The problem is that fujfilm_dynamic_range catches the post-import-image signal on RAF file import, and then only sets the exposure bias for the first of the duplicates.

You are right. post-import-image should be triggered for duplicates as well.
As you suggest, writing it to xmp could be a workaround, but I feel the previous solution better.

@dtorop
Copy link
Contributor Author

dtorop commented Apr 7, 2022

You are right. post-import-image should be triggered for duplicates as well.
As you suggest, writing it to xmp could be a workaround, but I feel the previous solution better.

@phweyland: Thanks for looking at this. I can take a look at fixing that in the C code, unless you're on top of that?

I also noticed that if I import an image with two XMP files, darktable tells the user that it imported 1 image. Which is technically true (one raw file was imported), but confusing (as two images are present in the lighttable view).

Is it necessary to involve lua to make appear the issue ?

I'm not sure. There is a route where it is read from the exif tag Exif.Photo.ExposureBiasValue or Exif.Image.ExposureBiasValue in exif.cc, but I haven't checked if that breaks on re-import. I don't know which cameras write this tag.

If yes, does that mean that the lua script calculates the bias ?

Yes, in the case of this script. It's a workaround. Ideally exiv2 could do this work within darktable's C import code. But last I checked it couldn't read the relevant tag. So instead the lua script does a (very slow) call to exiftool which can read the tag.

Ideal would be to get support for reading this tag into exiv2. I think it involves implementing a new datatype. I have this idea @kmilos works on exiv2 and would be able to speak to it better.

@kmilos
Copy link

kmilos commented Apr 7, 2022

exiv2 should have no problem reading Exif.Photo.ExposureBiasValue or Exif.Image.ExposureBiasValue, if they exist in the raw file. Since 0.27.4, there's even the exposureBiasValue() shorthand API that'll query both of these for you.

Not sure I understand what's the question here, sorry...

@dtorop
Copy link
Contributor Author

dtorop commented Apr 7, 2022

exiv2 should have no problem reading Exif.Photo.ExposureBiasValue or Exif.Image.ExposureBiasValue, if they exist in the raw file. Since 0.27.4, there's even the exposureBiasValue() shorthand API that'll query both of these for you.

Not sure I understand what's the question here, sorry...

There's one which Fujifilm writes that exiv2 can't read, even in v.0.27.5: RawExposureBias (0x9650). This represents the shift in EV for the chosen DR setting. With these cameras, even at 100DR ("standard") there is an EV shift:

  • 100 DR -> -0.72 EV
  • 200 DR -> -1.72 EV
  • 400 DR -> -2.72 EV

The tag is encoded as a 4-byte ratio of two signed shorts, which I don't think exiv2 can currently read.

There is also ab exiv2-readable DevelopmentDynamicRange tag which maps to RawExposureBias as above. DevelopmentDynamicRange is only present when tag DynamicRangeSetting (0x1402) is Manual/Raw (0x0001). When it
is Auto (0x0000), the equivalent data is tag AutoDynamicRange (0x140b). But exiv2 currently can't read that tag either.

So from exiv2 POV, it seems like there'd be two routes:

  1. Implement AutoDynamicRange with existing datatypes. Then have a heuristic in darktable to figure out the EV shift.
  2. Implement 4-byte ratio datatype and RawExposureBias as a general solution.

From darktable POV, the workaround is the lua script and calls to exiftool, which is 10x slower than exiv2.

@phweyland
Copy link

I can take a look at fixing that in the C code, unless you're on top of that?

I'm not on top of that. If I've worked on image_import(), the duplicate part remains a bit tricky for me. Please feel free to propose something !

@dtorop
Copy link
Contributor Author

dtorop commented Apr 7, 2022

I'm not on top of that. If I've worked on image_import(), the duplicate part remains a bit tricky for me. Please feel free to propose something !

Will take a look... I looked at it a year or so. I know there's been some good work on it recently -- and it's working much better.

@kmilos
Copy link

kmilos commented Apr 7, 2022

There's one which Fujifilm writes that exiv2 can't read, even in v.0.27.5: RawExposureBias (0x9650).

Ah, this one is in a completely separate block of the RAF file that exiv2 currently does not parse indeed.

@dtorop
Copy link
Contributor Author

dtorop commented Apr 8, 2022

There's one which Fujifilm writes that exiv2 can't read, even in v.0.27.5: RawExposureBias (0x9650).

Ah, this one is in a completely separate block of the RAF file that exiv2 currently does not parse indeed.

Interesting. Is it constructive to open an issue about this in the exiv2 project?

@kmilos
Copy link

kmilos commented Apr 8, 2022

It's already open, as linked. You might want to just chime in that you'd like this field to be parsed. No idea when someone will get around to implementing this though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants