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

Support Panasonic RW2 v7 formats #412

Merged
merged 6 commits into from
Jan 24, 2023
Merged

Conversation

jdneumeyer77
Copy link
Contributor

@jdneumeyer77 jdneumeyer77 commented Dec 14, 2022

Support 12 and 14bit V7 RW2 raw format.

I tested various files from my Panasonic S5 in various modes that produce this format:

  • Hi-res/Pixel-shift
  • Multiple exposure mode (like double exposures from film days)
  • Live Composite mode

I ported some code from Libraw. Plus, I cleaned it up a bit to make the bit manipulation easier to understand and came up with an easy way to support both 12 bit and 14 bit. My camera produces only 14 bit. Even Libraw mentions having never seen 12 bit in the wild.

This should address: darktable-org/darktable#8338 darktable-org/darktable#9768 #246 . It might fix others as well.

I believe there are already samples available. I mimicked the existing fuzzers, but I'm not sure how they work. Nor am I sure of other automated tests that could be enabled. Happy to add those.

@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Base: 59.02% // Head: 58.56% // Decreases project coverage by -0.47% ⚠️

Coverage data is based on head (578a44c) compared to base (602b204).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #412      +/-   ##
===========================================
- Coverage    59.02%   58.56%   -0.47%     
===========================================
  Files          221      223       +2     
  Lines        13354    13459     +105     
  Branches      1845     1851       +6     
===========================================
  Hits          7882     7882              
- Misses        5331     5436     +105     
  Partials       141      141              
Flag Coverage Δ
integration 46.44% <0.00%> (-0.39%) ⬇️
rpu_u 46.44% <0.00%> (-0.39%) ⬇️
unittests 21.63% <0.00%> (-0.18%) ⬇️

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

Impacted Files Coverage Δ
...rawspeed/decompressors/PanasonicV7Decompressor.cpp 0.00% <0.00%> (ø)
src/librawspeed/decoders/Rw2Decoder.cpp 58.37% <0.00%> (-1.53%) ⬇️
...rawspeed/decompressors/PanasonicV7Decompressor.cpp 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@LibRaw
Copy link
Contributor

LibRaw commented Dec 15, 2022

You ported our (LibRaw) code, please don't forget to mention it in copyright section

@kmilos
Copy link
Collaborator

kmilos commented Dec 15, 2022

I think one of the GH5S or something required v7 as well.

GH5M2 (darktable-org/darktable#9370) is actually v6, but w/ just 14b mode, while only 12b is currently implemented by rawspeed. GH5S is a v5 where dual 12b/14b is already supported.

@jdneumeyer77
Copy link
Contributor Author

jdneumeyer77 commented Dec 15, 2022

@LibRaw No worries. I can't take credit for reverse engineering that decodes the block. Feel free to use the readability improvements.

@kmilos You're right about the GH5M2, but I think it needs 12 bit support not 14 bit. I was thinking about tackling that issue as well since I have some extra free time. I don't own that camera, but I can test some samples and make sure my S5's files don't break. I thought there was a G* series camera that used the v7, but I think I'm mistaken. GH6 uses a new v8 format.

@jdneumeyer77 jdneumeyer77 changed the title Support Panasonic RW2 V7 formats Support Panasonic RW2 v7 formats Dec 15, 2022
@LibRaw
Copy link
Contributor

LibRaw commented Dec 15, 2022

@jdneumeyer77 This format is so simple that no reverse engineering was needed, just accurate look into RAW data.
Thank you for adding our copyright.

@LebedevRI
Copy link
Member

You ported our (LibRaw) code, please don't forget to mention it in copyright section

Does LGPL really require the Copyright (C) line to be present, not just the license as a whole?
Quote please?

@LibRaw
Copy link
Contributor

LibRaw commented Dec 15, 2022

I have not asked about this specific form, just about 'mentioning'
It is up to original code author how to mention LibRaw. If @jdneumeyer77 preferred to put (C) then we agree

@jdneumeyer77
Copy link
Contributor Author

jdneumeyer77 commented Dec 15, 2022

I was just following existing conventions. v6, which looks to be ported as well, had the Copyright (C) LibRaw LLC ([email protected]) already.

Good to know that's a simple format. I'm not familiar bayer sensor data. So I wouldn't know. I don't typically work with binary formats nor C++. I'll have to dig into it sometime to better understand what it is doing.

Edit: Ok I think I understand it clearer. now. Sensor data before this step is 14/12 bit pixels written out in big endian. It's read as byte chunks. In order to convert it the next 14/12 bits read and converted into little endian which is stored it in the next largest type size (2 bytes ushort).

@kmilos
Copy link
Collaborator

kmilos commented Dec 15, 2022

You're right about the GH5M2, but I think it needs 12 bit support not 14 bit.

It's quite possible that I mixed them up. In any case, one of the two is missing for V6.

@LebedevRI
Copy link
Member

@jdneumeyer77 so i'm finally looking into this, and i'm a bit troubled
by the amount of samples on RPU. Is this really only a single camera?

More importantly, in the original comment you wrote:

I tested various files from my Panasonic S5 in various modes that produce this format:

  • Hi-res/Pixel-shift
  • Multiple exposure mode (like double exposures from film days)
  • Live Composite mode

... but there is only a single (High resolution (sensor shift) mode 96 MP) affected sample.
Could you at least please contribute one more affected sample, one with minimal image size?

@LebedevRI LebedevRI self-assigned this Jan 24, 2023
@jdneumeyer77
Copy link
Contributor Author

@LebedevRI this is for several cameras. I believe S1 and S1H have the same features and use this format. The S1R uses a different sensor, but I think it uses this format as well for hi-res/live composite/etc.. I'm not sure if Gx series series uses this format for building images like hi-res mode.

I'll upload samples for live composite and multiple-exposure from my camera. I'll check if I can open hi-res samples from the S1/S1H/S1R to verify those as well. It's been working great for my camera the last couple weeks.

@LebedevRI
Copy link
Member

I'll upload samples for live composite and multiple-exposure from my camera.

At a bare minimum, that is indeed what i would like to see (on RPU) before merging this.
Mainly, i just want to have some smallest image with such format,
so i can tag it to be part of masterset, so it participates in perf testing etc.

It would be really good to have appropriate samples from other affected cameras.
Especially, if it suddenly turns out that one of them writes 12-bit raws...

@jdneumeyer77
Copy link
Contributor Author

@LebedevRI Ok. I've added two quick samples for live composite and multiple-exposure modes. They aren't much larger than normal raw files.

This issue indicates someone uploaded a high res sample for S1R. The sample doesn't appear on RPU. I did manually check if it's v7 format and it is with a sample I found elsewhere. It works locally. Same with a hi-res sample of the S1. No samples available, but I found a sample raw elsewhere. It also works.

It would be really good to have appropriate samples from other affected cameras. Especially, if it suddenly turns out that one of them writes 12-bit raws...

This PR includes support for both 12 bit and 14 bit. A comment from Libraw source code says they've not encountered a 12 bit file in this format yet.

@LebedevRI
Copy link
Member

@LebedevRI Ok. I've added two quick samples for live composite and multiple-exposure modes. They aren't much larger than normal raw files.

Thanks you, that'll do.

It would be really good to have appropriate samples from other affected cameras. Especially, if it suddenly turns out that one of them writes 12-bit raws...

This PR includes support for both 12 bit and 14 bit. A comment from Libraw source code says they've not encountered a 12 bit file in this format yet.

My point is that it's dead code if we don't have any evidence any such raws were ever produced.
So i'm just doing to rip it out. Which is why i'm asking to double check that other cameras don't produce them.

@pgassmann
Copy link

It would be really good to have appropriate samples from other affected cameras. Especially, if it suddenly turns out that one of them writes 12-bit raws...

I'm waiting for GH5M2 support which uses 12bit according to some comments.

Not sure if this is in the scope or where GH5M2 support is tracked. There's only one other issue here mentioning this camera: #367 and one for darktable: darktable-org/darktable#9370

@LebedevRI can you look at this too or does it make sense to create a separate issue?

@LebedevRI
Copy link
Member

As #367 mentions, that camera produces yet another format, v8, this adds support for v7.

@kmilos
Copy link
Collaborator

kmilos commented Jan 24, 2023

As #367 mentions, that camera produces yet another format, v8, this adds support for v7.

GH5M2 is v6, GH6 is v8...

@jdneumeyer77
Copy link
Contributor Author

@pgassmann GH5M2 uses v6 with 12 bit. I don't own the camera, but once I get time I can port that code over as well in a separate PR.

@LebedevRI I'm all for removing dead code, but it seems like Panasonic added 12 bit later for v6. Not sure if it was a similar case where there weren't any cameras that produced v6 12 bit at the time, so the support was excluded/ripped out.

Also I tried a few other tests such as using electronic shutter (ec) or ec + noise reduction with hi-res mode to see if they produced 12 bit. I know some manufactures bin to 12 bit using electronic shutter mode. Still produced 14 bit raws. So at least with my camera I can't produce 12 bit. I'll ask a friend to test and provide examples from S1R in different modes like multiple-exposure and using ec.

@LebedevRI
Copy link
Member

Thanks!
Here it will be trivial to add should the need emerge.

@LebedevRI LebedevRI merged commit 5822e55 into darktable-org:develop Jan 24, 2023
@LebedevRI
Copy link
Member

@jdneumeyer77 thank you! This should not have taken this long.

@pgassmann
Copy link

@pgassmann GH5M2 uses v6 with 12 bit. I don't own the camera, but once I get time I can port that code over as well in a separate PR.

Thank you. How much time does it take to port that code over? Is it worth a speparate issue to track this in rawspeed or is darktable-org/darktable#9370 enough? That one is currently assigned to @LebedevRI

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

Successfully merging this pull request may close these issues.

5 participants