Do not throw if DNG crop is undefined (DJI bug)

[kmilos]

The file is probably usable, so fall back to default values and log error instead.

This should address https://github.com/darktable-org/darktable/issues/11949 and https://discuss.pixls.us/t/unable-to-import-drone-hyperlapse-images/37222

Asked for samples...

[LebedevRI]

Yeah i do not know about this... I guess they expect us to treat such "rational"s w/ zero denominator by pretending that denominator is 1? But i really don't see why that is what the TIFF spec prescribes, and as far as i can tell neither dcraw nor libTIFF does that.

This is quite an unfortunate situation.

[kmilos]

Yeah i do not know about this... I guess they expect us to treat such "rational"s w/ zero denominator by pretending that denominator is 1?

I don't think we can assume that. For example, DJI really put (0/0, 0/0) in there, so for DefaultCropSize we can't and should not pretend it is (0,0).

But i really don't see why that is what the TIFF spec prescribes, and as far as i can tell neither dcraw nor libTIFF does that.

The Exif (or TIFF/EP) spec mentions for some fields that 0/0 means "undefined".

This is quite an unfortunate situation.

It is unfortunate, but I think falling back to default (entire image) is sensible in this specific case, instead of bailing out on the user. We do something similar already if the opcodes cannot be properly parsed (i.e. just ignore them, but not bail out)...

[kmilos]

Yep, the Exif spec mentions in several places:

This value is invalid when 0/0 is recorded.

[kmilos]

This doesn't work yet btw, because mRaw->dim is (0,0) at this point?!

[kmilos]

Argh, they also put ActiveArea as 0 0 0 0. Wonderful!

[LebedevRI]

Why do we care to support those invalid files?

[LibRaw]

To allow users to process files captured by their (invalid) equipment

[LebedevRI]

Also, was the bug reported to the DJI? cc @DJI-SDK @lanyusea

[kmilos]

Also, was the bug reported to the DJI?

cc @mluckau @JooB1 @WiLars @MMCMA @da-phil @cdhodgdon @fran6t

[codecov[bot]]

Codecov Report

Patch coverage: 60.86% and project coverage change: -0.02 :warning:

Comparison is base (834caca) 59.06% compared to head (c44a6ad) 59.04%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #475      +/-   ##
===========================================
- Coverage    59.06%   59.04%   -0.02%     
===========================================
  Files          232      232              
  Lines        13813    13824      +11     
  Branches      1932     1933       +1     
===========================================
+ Hits          8159     8163       +4     
- Misses        5521     5528       +7     
  Partials       133      133              

Flag	Coverage Δ
benchmarks	8.29% <0.00%> (-0.01%)	:arrow_down:
integration	47.35% <60.86%> (-0.01%)	:arrow_down:
linux	56.85% <60.86%> (-0.02%)	:arrow_down:
macOS	18.85% <0.00%> (-0.02%)	:arrow_down:
rpu_u	47.35% <60.86%> (-0.01%)	:arrow_down:
unittests	18.28% <0.00%> (-0.02%)	:arrow_down:
windows	∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/librawspeed/decoders/DngDecoder.cpp	52.96% <60.86%> (-0.39%)	:arrow_down:

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[lanyusea]

Also, was the bug reported to the DJI? cc @lanyusea

let me check

[kmilos]

Looks like both DefaultCropSize and ActiveArea are fixed in the recently launched DJI Air 3, but I guess there's still a need to support older invalid files.