darktable-org / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
9.08k stars 1.1k forks source link

Lens detection based on Exif.Photo.LensModel #16303

Open mingos777 opened 4 months ago

mingos777 commented 4 months ago

Is your feature request related to a problem? Please describe. Not a problem, rather an inconvenience. When listing my photos based on the lens used, I see photos taken with "Manual Lens No CPU". This limits my ability to filter my photo collection.

Describe the solution you'd like It would be great if Darktable allowed to list photos based on the contents of the Exif.Photo.LensModel tag instead of what exiv2 makes out of the Maker Notes. There could either be a setting to turn this on and off or some rules to prioritise which source to prefer, depending on what's easier to implement.

Alternatives I can't think of any.

Additional context This is what my collection (by lens) looks like. It's not exactly easy to filter, considering I have photos taken with 50+ different manual lenses.

Screenshot 2024-02-12 at 12 23 18

I always add the lens information manually so I know for a fact that it's there. Whenever I export a jpeg with the metadata, the Exif.Photo.LensModel is also put in there and other tools, such as Photos Exif Editor, or online services, like Flickr, recognise this tag and use it (example).

wpferguson commented 4 months ago

I always add the lens information manually

You're adding it to Exif.Photo.LensModel manually? Is this done prior to import?

kmilos commented 4 months ago

Although I see your point, one has to thread very carefully here, precisely because lens detection is a general pita. 👿

For example, changing priority risks breaking the lens correction module, because the separate lensfun database itself is constructed from exiv2/exiftool lens strings which usually come from hand-crafted internal interpretations of MakerNotes... Exif.Photo.LensModel was not (sanely) populated by most/major vendors for a very long time, so we're a bit stuck with the vicious circle resulting from that legacy...

mingos777 commented 4 months ago

@wpferguson

You're adding it to Exif.Photo.LensModel manually? Is this done prior to import?

Yes, I add lens information and aperture information (if I have it written down or memorised). I have a couple scripts that use exiv2 to batch add this info with minimal input from me. Super useful. And yes, I do it prior to importing to darktable.

@kmilos I understand the risks and this is why I suggested making this configurable. Current behaviour could be the default one. But people like me, who photograph primarily with manual lenses, likely wouldn't use the lens correction anyway due to an almost complete lack of vintage glass in lensfun database (or at least in what appears in dt's lens choice dropdown in the module). OTOH, being able to filter by lens would be an improvement.

Alternatively, filtering by Exif.Photo.LensModel or an arbitrary EXIF property would also serve the same purpose and wouldn't break any functionality that depends on correct lens detection.

wpferguson commented 4 months ago

I was thinking of a lua-script solution where we copy the Exif.Photo.LensModel to exif_lens field in the database.

It could be set up with a post-import trigger if all the data is in place prior to import, otherwise it could be run on a selection/collection with a keyboard shortcut.

kmilos commented 4 months ago

this is why I suggested making this configurable

Sure, was just thinking of the fallout here - I'm not sure everyone would understand the consequences of this option unless we start throwing up some big red UI warnings when this is changed from default. (And we would have to deal w/ new complaints etc.)

The request makes sense, just saying it needs to be thought through really well and managed carefully.

wpferguson commented 4 months ago

Alternatively, filtering by Exif.Photo.LensModel or an arbitrary EXIF property

Filtering takes place on information in the database

This probably effects less than 1 percent of the images imported into darktable, so therefore it's an edge case. I don't think it should be implemented in master, especially with all the possibilities for negative consequences.

mingos777 commented 4 months ago

I was thinking of a lua-script solution where we copy the Exif.Photo.LensModel to exif_lens field in the database.

Cumbersome but it would do the job nicely.

Sure, was just thinking of the fallout here - I'm not sure everyone would understand the consequences of this option unless we start throwing up some big red UI warnings when this is changed from default. (And we would have to deal w/ new complaints etc.)

If it's a potentially feature-breaking one, then yes.

How about a new field in the database that would hold the value of Exif.Photo.LensModel? That shouldn't break anything and would make photo filtering possible.

mingos777 commented 4 months ago

Anyway, I'll leave this request for you to ponder it and decide whether to do anything with it or not. In the meantime, I'll look into learning some Lua.

wpferguson commented 4 months ago

@mingos777 could you send me a couple of images (or a link) so that I can play with a script?

mingos777 commented 4 months ago

Here.

10 photos, 5 lenses (one with a CPU and four manual). I hope this is sufficient.

wpferguson commented 4 months ago

That works, thanks.

What fields do you set besides Exif.Photo.LensModel? Exif.Photo.FNumber?

mingos777 commented 4 months ago

From my Automator script:

# update exif data in files
for FILE in "$@"
do
    /opt/homebrew/bin/exiv2 -M"set Exif.Photo.LensModel $LENS[1]" \
                            -M"set Exif.Photo.LensMake $LENS[2]" \
                            -M"set Exif.Photo.LensSerialNumber $LENS[3]" \
                            -M"set Exif.Photo.FocalLength $LENS[4]/1" \
                            -M"set Exif.Photo.FocalLengthIn35mmFilm $LENS[4]" \
                            -M"set Exif.Photo.LensSpecification $LENS[6]" \
                            $FILE
done

I think most of the files I sent you will not have Exif.Photo.LensSpecification set as it's something I only added recently.

And yes, in a separate Automator action, I also set Exif.Photo.FNumber.

wpferguson commented 4 months ago

I've attached a script

populate_lensdata.zip

If you don't have the lua-scripts installed, use the scripts installer at the bottom left panel in lighttable to install them.

Unzip the file and place the script in the contrib folder (~/.config/darktable/lua/contrib or %LOCALAPPDATA\darktable\lua\contrib).

In script_manager go to the contrib category, find populate_lensdata and start it. It uses exiv2 to obtain the information from the image file so you'll need that installed for the script to work.

In preferences go to shortcuts->Lua scripts and you can assign shortcuts to the collection and selection functions so that you can take care of the already imported files.

The script only replaces Manual Lens No CPU strings with the data it finds. If it doesn't find data, then it leaves the Manual Lens No CPU

wpferguson commented 4 months ago

I didn't see your reply with all the things you set until after I had sent you the script. I'll update it

wpferguson commented 4 months ago

I looked at the other fields you're loading and they are already being populated correctly or don't have an equivalent field in the image structure to populate so I didn't end up adding any more fields. I did however, fix a bug so here's an updated version.

populate_lensdata.zip

mingos777 commented 4 months ago

Judging by the debug output, the script fails when the path to the files contains a space.

EDIT: Changing line 70 of your script seems to have done the trick. Now the lens data is populated correctly both on import and when I use the assigned keyboard shortcut to process the selection:

  local cmd = pl.exiv2 .. " -g Photo.LensModel" .. " \"" .. image.path .. PS .. image.filename .. "\""

Thank you @wpferguson for the help. As far as my needs go, I am satisfied with this solution. I'll leave the decision to you whether this feature request should be closed or not.

wpferguson commented 4 months ago

I fixed it a slightly different way, but your way works fine too...

populate_lensdata.zip

I'll leave this open until I add the script to the lua-scripts repository.

mingos777 commented 4 months ago

I added one more photo to the gdrive folder. The script seems to misbehave with it and blanks out the lens field altogether. I assume that the condition if lensdata resolves to true - maybe there's whitespace or an endline character there.

wpferguson commented 4 months ago

if lensdata resolves to true

lensdata exists, but it's an empty string.

Changed get_lens_data() to strip off trailing return and linefeed and return nil if the string is empty. Also added some logging in case somebody has a question about what is happening.

populate_lensdata.zip

github-actions[bot] commented 2 months ago

This issue has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

mingos777 commented 1 month ago

@wpferguson I've just noticed something about the script that might need fixing. I imported some photos with duplicates into darktable and the lensdata script only processed the first photo of each set of duplicates. For the remaining ones, I had to select, use the populate_lensdata shortcut, leave the collection and get back to it - as many times as there were duplicates.

wpferguson commented 1 month ago

@mingos777 where the groups collapsed when only the first image of a group got processed?

mingos777 commented 1 month ago

@wpferguson Yes, they were. I have my groups collapsed by default.

Additionally, I think opening images with a space in the filename is problematic:

/Volumes/Zdjęcia/Pentax/NZ7_2683: Failed to open the file
f2.8.NEF: Failed to open the file
   158,4562 LUA ERROR : ...ngos/.config/darktable/lua/contrib/populate_lensdata.lua:78: bad argument #1 to 'gsub' (string expected, got nil)

Also, the same error is thrown when attempting to process an image that is in dt database but was actually deleted from the disc.

Anyway, for now, I located deleted images and removed them from the library before running the script on the remainder. I modified the get_lens_data function to account for the other errors I saw:

local function get_lens_data(image)
  local cmd = pl.exiv2 .. " -g Photo.LensModel" .. " " .. ds.sanitize(image.path) .. PS .. "\"" .. image.filename .. "\""
  local p = io.popen(cmd)
  local lensdata = nil
  if p then
    local data = p:read("*all")
    p:close()
    lensdata = string.match(data, "Exif.Photo.LensModel%W+Ascii%W+%d+%W+(.*)$")
    if lensdata then
      lensdata = string.gsub(lensdata, "\n", "")
      lensdata = string.gsub(lensdata, "\r", "")
    end
    if string.len(lensdata) == 0 then
      dt.print_log("Exif.Photo.LensModel did not contain any information")
      lensdata = nil 
    end
  end
  return lensdata
end