Crop Panasonic G9 images using vendor metadata

[jsmucr]

This is supposed to fix #170 and help prepare other related fixes.

Now the decoders can decide whether to supply their own crop data when the Crop tag in a camera.xml entry is empty or absent. Rw2Decoder has been updated to leverage this new ability, and the Crop tags for Panasonic G9 entries have been removed.

[LebedevRI]

Github suggests this isn't rebased properly.

[jsmucr]

Github suggests this isn't rebased properly.

I know. My original intention has been to test it against DT 4.2.1 because that's what I use, hence the rebase. I'll put it back later.

[jsmucr]

Ok, it appears to work as intended with DT 4.2.1. Rebased back onto develop.

[codecov]

Codecov Report

Patch coverage: 75.00% and project coverage change: -0.02 ⚠️

Comparison is base (c65fdca) 59.07% compared to head (302a284) 59.05%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #490      +/-   ##
===========================================
- Coverage    59.07%   59.05%   -0.02%     
===========================================
  Files          232      232              
  Lines        13888    13912      +24     
  Branches      1945     1951       +6     
===========================================
+ Hits          8204     8216      +12     
- Misses        5551     5565      +14     
+ Partials       133      131       -2     

Flag	Coverage Δ
benchmarks	8.27% <20.45%> (+0.01%)	⬆️
integration	47.51% <86.84%> (-0.01%)	⬇️
linux	56.97% <84.61%> (+<0.01%)	⬆️
macOS	18.75% <18.18%> (-0.01%)	⬇️
rpu_u	47.51% <86.84%> (-0.01%)	⬇️
unittests	18.19% <20.45%> (-0.01%)	⬇️
windows	∅ <ø> (?)

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

Impacted Files	Coverage Δ
src/librawspeed/decoders/RawDecoder.h	66.66% <ø> (ø)
src/librawspeed/decoders/Rw2Decoder.h	100.00% <ø> (ø)
src/librawspeed/metadata/Camera.h	66.66% <ø> (ø)
src/librawspeed/decoders/Rw2Decoder.cpp	57.77% <69.56%> (+1.46%)	⬆️
src/librawspeed/decoders/RawDecoder.cpp	36.47% <72.72%> (+0.10%)	⬆️
src/librawspeed/metadata/Camera.cpp	74.78% <90.00%> (+0.22%)	⬆️

... and 32 files with indirect coverage changes

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

[LebedevRI]

Thanks for looking into this!

I do think something like this is needed, but:
this perhaps needs to be more fine-grained,
and perhaps less fine-grained, i.e.:

• Should this only apply to the problematic mode?
• is this really the only affected camera?
• Should we just always use this vendor crop for panasonic?

(I don't know the answer to these questions, this isn't a recommendation.)

Thoughts?

[jsmucr]

The most problematic thing here is in fact to figure out the mode, as here are many combinations of how you can shoot. What I was addressing here was primarily my issue with hi-res pixel shift mode and some of the burst modes. I wasn't able to figure it out just by looking at the metadata. I would have to take the camera and literally try out every single setting to make a note about the crop. And then maybe a software update would come out and I would have to start over to make sure they didn't change anything related. And even then I could get it wrong without some serious pixel peeping.

It's definitely not the only camera affected. There was this issue with a Panasonic camera, I think LX7, which seemed to crop raw files when using digital zoom.
But generally, I was very surprised to see there's a database of cameras with reverse engineered crops for each mode (whatever that means). I myself wouldn't dare to override vendor settings unless necessary.

I can only provide knowledge regarding the G9 and the GX80 Panasonic cameras. And lend a hand in checking the available samples of the other models. Until then, falling back to vendor settings could provide a fast hotfix whenever someone comes up with a similar issue.

[jsmucr]

Will this get merged, or shall I do something more with this fix?

[LebedevRI]

Thank you!
Sorry for the silence, i suppose this is better than what we do now :)

High-level, the particular approach feels somewhat wrong,
i mean, how do those with applyCrop = false deal with this?
But that is not a blocker i suppose.