Skip to content

First revision of Panasonic RW2 version 8 Decompressor #790

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
May 13, 2025
Merged

First revision of Panasonic RW2 version 8 Decompressor #790

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
917b1b6
First revision of Panasonic RW2 version 8 decoder
kjyager Apr 3, 2025
abbcb37
Removing list length limit because I realized it is unnecessary.
kjyager Apr 3, 2025
be38909
Resolved warnings treated as errors
kjyager Apr 3, 2025
bdff0f0
Resolving clang warnings treated as errors
kjyager Apr 3, 2025
f9441ee
Drop spurious spaces, apply clang-format
LebedevRI Apr 4, 2025
3c87e16
`uint` is not a type
LebedevRI Apr 4, 2025
5f2e74c
Copied G9M2 entries from kmilos/panny_g9m2
kjyager Apr 4, 2025
0a63255
Merge remote-tracking branch 'upstream/develop' into kyager_rw2_dev
LebedevRI Apr 5, 2025
cb25e4b
Re-apply clang-format
LebedevRI Apr 5, 2025
e3518cf
`UINT16_WIDTH` is not a thing
LebedevRI Apr 5, 2025
8725ab5
Update to support vertical strips.
kjyager Apr 30, 2025
9f25f5f
Merge remote-tracking branch 'upstream/develop' into kyager_rw2_dev
LebedevRI May 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion data/cameras.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11547,8 +11547,25 @@
</ColorMatrix>
</ColorMatrices>
</Camera>
<Camera make="Panasonic" model="DC-G9M2" supported="no">
<Camera make="Panasonic" model="DC-G9M2">
<ID make="Panasonic" model="DC-G9M2">Panasonic DC-G9M2</ID>
<ColorMatrices>
<ColorMatrix planes="3">
<ColorMatrixRow plane="0">8325 -3456 -623</ColorMatrixRow>
<ColorMatrixRow plane="1">-4330 12089 2528</ColorMatrixRow>
<ColorMatrixRow plane="2">-860 2646 5984</ColorMatrixRow>
</ColorMatrix>
</ColorMatrices>
</Camera>
<Camera make="Panasonic" model="DC-G9M2" mode="4:3">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More work on crop modes probably needs to be done. RW2 V8 cameras don't seem to crop into raw images the way the old Panasonic cameras did.

Copy link
Collaborator

@kmilos kmilos Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, only the older ones seem to actually crop the raw... I'm hoping it'll work out the same as v7, so I just have the "native" sensor crop/aspect in cameras.xml for now. (We never supported the "soft" crop anyway...)

I should be able to help w/ testing this a bit later. We'll need check any other non-standard modes though like high-res etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your concerns w.r.t high-res mode? I've tested on plenty of high-res shots and not seen any apparent issues.

Copy link
Collaborator

@kmilos kmilos Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your concerns w.r.t high-res mode?

Typically if we have a manual crop value other than (0,0,0,0) in cameras.xml for native mode it might not apply (i.e. crop too much or too little) in high-res mode for some other brands or codec (i.e. sensor and processor) versions... It'll just have to be checked out, that's all... Same for any other sensor readout modes like burst, live composite, multi-exposure etc. (present in earlier models like the S5, don't know if they are also in newer ones).

Copy link
Collaborator

@kmilos kmilos Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My (very) cursory crop tests so far, using LibRaw's unprocessed_raw dumps for "native" mode samples only:

GH6: (0,0,0,0)
S5M2: (0,0,0,0)
S5M2X: (0,0,0,0)
G9M2: (0,0,0,0)
S9: (0,0,0,0)
GH7: (0,0,0,0)
S1RM2: (0,0,0,0)

These are w.r.t. sizes in e.g. these tags as reported by exiftool -a -u -s -G1 -H:

[IFD0]          0x0002 SensorWidth
[IFD0]          0x0003 SensorHeight

Copy link
Collaborator

@kmilos kmilos Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, if you're excluding the <Crop x="0" y="0" width="0" height="0"/> like you have here, one should be getting the crop from vendor metadata tags (0x0004 SensorTopBorder, 0x0005 SensorLeftBorder, 0x0006 SensorBottomBorder, 0x0007 SensorRightBorder), so somewhat smaller than the full sensor...

Copy link
Contributor Author

@kjyager kjyager Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not sure I'm following you, but happy to make any code changes needed.

Is this not covered by Rw2Decoder::getDefaultCrop()? That seems to be checking the appropriate tags. Are you suggesting that a further crop, on top of the sensor crop, may be necessary for files produced by the other modes?

Edit: On second look, Rw2Decoder::getDefaultCrop() doesn't seem to be called anywhere. Calling mRaw->subFrame(getDefaultCrop()); before returning the raw from Rw2Decoder::decodeRawInternal() seems to correctly apply the crop. @kmilos Would you like me to push this change, or did you have something else in mind?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This basically boils down to the choice between the vendor crop from metadata, or manual crop values from cameras.xml. Traditionally RawSpeed had a design goal mentioned on the landing page: "RawSpeed does NOT crop the image to the same sizes as manufactures, but supplies biggest possible images." This was foregone for some vendors/models, as it was becoming unwieldy to check, list and handle explicitly all possible modes via cameras.xml.

For the time being, I suggest to implement it like in the v5/v6 decoder: only call getDefaultCrop() if <Crop/> was not present and restore <Crop x="0" y="0" width="0" height="0"/> in cameras.xml v8 model entries as those seem to be "well behaved" so far...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see. I can add that. Using zero crop on the images I've looked at does reveal some smeary pixels at the edges of the frame. I don't think it is an issue, but want to point it out in-case that biggest image design sentiment has changed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add that.

Great!

Using zero crop on the images I've looked at does reveal some smeary pixels at the edges of the frame.

Let's check it out more closely for all models during this review and then either adjust the manual <Crop/> accordingly, or simply drop it for some/all v8 finally.

<ID make="Panasonic" model="DC-G9M2">Panasonic DC-G9M2</ID>
<ColorMatrices>
<ColorMatrix planes="3">
<ColorMatrixRow plane="0">8325 -3456 -623</ColorMatrixRow>
<ColorMatrixRow plane="1">-4330 12089 2528</ColorMatrixRow>
<ColorMatrixRow plane="2">-860 2646 5984</ColorMatrixRow>
</ColorMatrix>
</ColorMatrices>
</Camera>
<Camera make="Panasonic" model="DC-G95">
<ID make="Panasonic" model="DC-G95">Panasonic DC-G95</ID>
Expand Down
2 changes: 1 addition & 1 deletion src/librawspeed/bitstreams/BitStreamer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ BitStreamerReplenisherBase<Tag>::establishClassInvariants() const noexcept {
}

template <typename Tag>
struct BitStreamerForwardSequentialReplenisher final
struct BitStreamerForwardSequentialReplenisher
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to remove final to implement BitStreamerReversedSequentialReplenisher in PansonicV8Decompressor.cpp. See comments around that class for more context.

: public BitStreamerReplenisherBase<Tag> {
using Base = BitStreamerReplenisherBase<Tag>;
using Traits = BitStreamerTraits<Tag>;
Expand Down
11 changes: 11 additions & 0 deletions src/librawspeed/decoders/Rw2Decoder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "decompressors/PanasonicV5Decompressor.h"
#include "decompressors/PanasonicV6Decompressor.h"
#include "decompressors/PanasonicV7Decompressor.h"
#include "decompressors/PanasonicV8Decompressor.h"
#include "decompressors/UncompressedDecompressor.h"
#include "io/Buffer.h"
#include "io/ByteStream.h"
Expand Down Expand Up @@ -170,6 +171,16 @@ RawImage Rw2Decoder::decodeRawInternal() {
v7.decompress();
return mRaw;
}
case 8: {
// Most use bps=16, S9 uses bps=12
if (bitsPerSample != 16 && bitsPerSample != 12)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll probably remove this check. The BPS for V8 files seems to vary a lot, but should all decode fine.

ThrowRDE("Version %i: unexpected bits per sample: %i", version,
bitsPerSample);
PanasonicV8Decompressor v8(mFile, mRaw, *raw);
mRaw->createData();
v8.decompress();
return mRaw;
}
default:
ThrowRDE("Version %i is unsupported", version);
}
Expand Down
2 changes: 2 additions & 0 deletions src/librawspeed/decompressors/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ FILE(GLOB SOURCES
"PanasonicV6Decompressor.h"
"PanasonicV7Decompressor.cpp"
"PanasonicV7Decompressor.h"
"PanasonicV8Decompressor.cpp"
"PanasonicV8Decompressor.h"
"PentaxDecompressor.cpp"
"PentaxDecompressor.h"
"PhaseOneDecompressor.cpp"
Expand Down
Loading
Loading