fix wrong month in EXIF.MONTH.LONG and EXIF_MONTH.SHORT when no exif date available in image

[deekayhd]

PR #594 fixed a bug in rename_images.lua with images that have no EXIF datetime. However, the fix (setting datetime_taken to 0000.00.00) does not work on Windows, because the date is smaller than what can be represented on this platform.
In addition, on linux the date 0000.00.00 results in EXIF.MONTH.LONG and EXIF.MONTH.SHORT as "November" and "Nov", respectively.
In this PR I propose to skip the calculation of EXIF.MONTH.LONG and .SHORT for images without a date, and set them as follows:
EXIF.MONTH.LONG = "unknown"
EXIF.MONTH.SHORT = "???"

closes issue #627

[wpferguson]

The default for string substitution is that we return an empty string if we can't come up with a substitution.

set them as follows:
EXIF.MONTH.LONG = "unknown"
EXIF.MONTH.SHORT = "???

Set them to "". That way if they are used in a substitution, especially for a filename, they just end up as nothing and don't interfere. You could do a log.msg(log.warn, "... saying there wasn't a time preset so it was filled with empty strings.

[deekayhd]

That way if they are used in a substitution, especially for a filename, they just end up as nothing and don't interfere.

ok, will do that. My thinking was kind of the opposite: If they are used, e.g. as a folder name, a blank variable would disturb the pattern, hierarchy, etc.
Should we then also use "" for the other variables derived from datetime_taken, like EXIF.YEAR, EXIF.MONTH, etc (currently set to "0000" and "00")?

[wpferguson]

Should we then also use "" for the other variables derived from datetime_taken, like EXIF.YEAR, EXIF.MONTH, etc (currently set to "0000" and "00")?

I think so. It helped above when I said that about the default so now I can look at this with that in mind. When I was fixing it, I didn't think about the big picture.

[deekayhd]

Changes done as you requested.
One question about the logging: The default log level in string.lua is error, so a warning is never shown unless the user changes the source code. Is this intended or am I missing something? Is there a way to override the defaul log level?

[wpferguson]

You can set it from a script

local ds = require "lib/dtutils.string
local log = require "lib/dtutils.log
ds.log_level = log.warn

Or, we could keep it hidden in the library and add getter and setter functions in the library to manipulate the log level

[deekayhd]

You can set it from a script

local ds = require "lib/dtutils.string
local log = require "lib/dtutils.log
ds.log_level = log.warn

Or, we could keep it hidden in the library and add getter and setter functions in the library to manipulate the log level

Would be nice if there was an option one could specify when starting darktable, like
darktable --d lua_log_level warn

[wpferguson]

I cannot test on Windows. Could it be different there?

Shouldn't be. It comes from the image, not the operating system.

Would be nice if there was an option one could specify when starting darktable, like
darktable --d lua_log_level warn

I already have in mind a script that lets you change the log level of another script on the fly, so if something isn't behaving and you're already running with -d lua you could just look at the terminal output.

Starting darktable --d lua_log_level warn would make all the scripts go to that log level and then all the scripts logging would bury you in output. Better a "targeted" approach.

[deekayhd]

It comes from the image, not the operating system.

And there is darktable core in between. If I understand the coding correctly, the value is set to an empty string, if no tag is found:

static void _find_datetime_taken(Exiv2::ExifData &exifData,
                                 Exiv2::ExifData::const_iterator pos,
                                 char *exif_datetime_taken)
{
  // Note: We allow a longer "datetime original" field with an unnecessary
  // trailing byte(s) due to buggy software that creates it.
  // See https://github.com/darktable-org/darktable/issues/17389
  // We also accept the out of spec Exif.Photo.DateTimeOriginal
  // without a null terminator, which AnalogExif creates.
  // See https://github.com/darktable-org/darktable/issues/18146
  if((FIND_EXIF_TAG("Exif.Image.DateTimeOriginal")
      || FIND_EXIF_TAG("Exif.Photo.DateTimeOriginal"))
     && pos->size() >= DT_DATETIME_EXIF_LENGTH - 1)
  {
    _strlcpy_to_utf8(exif_datetime_taken, DT_DATETIME_EXIF_LENGTH, pos, exifData);
    if(FIND_EXIF_TAG("Exif.Photo.SubSecTimeOriginal")
       && pos->size() > 1)
    {
      char msec[4];
      _strlcpy_to_utf8(msec, sizeof(msec), pos, exifData);
      dt_datetime_add_subsec_to_exif(exif_datetime_taken, DT_DATETIME_LENGTH, msec);
    }
  }
  else
  {
    *exif_datetime_taken = '\0';
  }
}

[wpferguson]

If I understand the coding correctly

Yes. That's consistent across platforms. In practice you almost never run into it, so it's mostly a corner case. The substitution code went a long time before anyone managed to find the bug. My solution wasn't very well thought out.

[wpferguson]

@deekayhd Everything looks good to me, are we good to merge?

[deekayhd]

@wpferguson From my perspective, we can merge!
Thanks for checking!