First revision of Panasonic RW2 version 8 Decompressor #790

kjyager · 2025-04-03T03:04:04Z

Adapted from the Rust implementation in @cytrinox's dnglab. Should make it possible to provide support for several newer Panasonic cameras: G9M2, S5II, S1RII, GH6, GH7, and S9.

In this branch, cameras.xml has only been updated for the G9M2.

@cytrinox

Adapted from the Rust implementation in @cytrinox [dnglab](https://github.com/dnglab/dnglab). Should make it possible to provide support for several newer Panasonic cameras: G9M2, S5II, S1RII, GH6, GH7, and S9. In this branch, cameras.xml has only been updated for the G9M2.

kjyager · 2025-04-03T03:05:14Z

src/librawspeed/bitstreams/BitStreamer.h

@@ -71,7 +71,7 @@ BitStreamerReplenisherBase<Tag>::establishClassInvariants() const noexcept {
 }

 template <typename Tag>
-struct BitStreamerForwardSequentialReplenisher final
+struct BitStreamerForwardSequentialReplenisher


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

kjyager · 2025-04-03T03:06:12Z

data/cameras.xml

+			</ColorMatrix>
+		</ColorMatrices>
+	</Camera>
+	<Camera make="Panasonic" model="DC-G9M2" mode="4:3">


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.

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.

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.

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).

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

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...

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?

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...

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.

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.

kjyager · 2025-04-03T03:07:49Z

src/librawspeed/tiff/TiffTag.h

-  CURRENTPREPROFILEMATRIX = 50834,
-  COLORIMETRICREFERENCE = 50879,
-  KODAKKDCPRIVATEIFD = 65024,
+  SHADOWSCALE = 0XC633,


Making use of hex values consistent for the enum, and grouped Panasonic tags together with the new ones. Hopefully not overstepping.

kjyager · 2025-04-03T03:24:18Z

src/librawspeed/decompressors/PanasonicV8Decompressor.cpp

+// The set of templates and classes below define a specialized bit streamer which works mostly the same as 
+// BitStreamerMSB, but which reverses the ordering of the bits within each byte. 


The Panasonic files require streaming bits in a way which seems unsupported by the existing utilities. The bit-stream consumes bytes in MSB order, but reads the bits within each byte starting from the least-significant bits. Likewise a version of BitStreamerMSB is needed which reverses bit order while caching bits from bytes. This is what I've done below, hacking together a BitStreamerReversedSequentialReplenisher struct which is the same as BitStreamerForwardSequentialReplenisher but which copies reversed bits within getInput()

I'm not at all happy with this solution, but it is the only solution I could come up with that didn't require writing a new bit pump implementation from scratch or radically changing existing bit streamer code.

In my opinion, the templating of the bit streaming classes make them very difficult to extend or modify without writing lots of new code. This is the briefest thing I could come up with, and it still feels over-verbose. I would greatly appreciate any recommendations on how to better handle this.

kjyager · 2025-04-03T03:28:02Z

src/librawspeed/decompressors/PanasonicV8Decompressor.cpp

+}
+
+/// Maybe the most complicated part of the entire file format, and seemingly, completely unused. 
+void PanasonicV8Decompressor::populateGammaLUT(const TiffIFD& ifd) {


The code in this function looks radically different from what is in the original implementation because I've done my best to reverse engineer the original intent, and to reduce it down into something shorter and more intelligible.
I'm fairly confident that this version is logically equivalent to the original, but since no RW2 file has ever been found which actually uses this code, it is difficult to know for sure. Caution should be taken if this code ever ends up in use.

I had missed these because I have been building the library through Darktable.

kjyager · 2025-04-03T19:12:49Z

src/librawspeed/decoders/Rw2Decoder.cpp

@@ -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)


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

* upstream/develop: .. but that directory isn't actually empty, it just doesn't have files CI: skip step if no clang-tidy YAML reports CI: run CodeQL static analysis even if some steps failed CI: disable `-Werror` for static analysis jobs Github: sarif-ify clang-tidy output CMake: add `RAWSPEED_ENABLE_CLANG_TIDY_WERROR` option Explicitly opt-out of `alpha.unix.Stream` CSA check rstest: add (some) missing checks that writing succeeded `FileReader::readFile()`: first check for erorr, then eof Upload CodeChecker results as SARIF Switch RPU masterset fetching to wget Add Nikon Z5_2 placeholder

kjyager added 2 commits April 2, 2025 20:00

Removing list length limit because I realized it is unnecessary.

abbcb37

kjyager commented Apr 3, 2025

View reviewed changes

kjyager marked this pull request as ready for review April 3, 2025 03:31

kjyager requested a review from LebedevRI as a code owner April 3, 2025 03:31

kjyager changed the title ~~First revision of Panasonic RW2 version 8 decoder~~ First revision of Panasonic RW2 version 8 Decompressor Apr 3, 2025

kjyager mentioned this pull request Apr 3, 2025

Panasonic DC-GH6 raws are version 8 - new decompressor needed #367

Open

kjyager added 2 commits April 2, 2025 20:54

Resolved warnings treated as errors

be38909

I had missed these because I have been building the library through Darktable.

Resolving clang warnings treated as errors

bdff0f0

kjyager commented Apr 3, 2025

View reviewed changes

LebedevRI added 2 commits April 4, 2025 07:20

Drop spurious spaces, apply clang-format

f9441ee

uint is not a type

3c87e16

LebedevRI force-pushed the kyager_rw2_dev branch from e204089 to 3c87e16 Compare April 4, 2025 04:21

kjyager and others added 4 commits April 4, 2025 01:38

Copied G9M2 entries from kmilos/panny_g9m2

5f2e74c

Re-apply clang-format

cb25e4b

UINT16_WIDTH is not a thing

e3518cf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First revision of Panasonic RW2 version 8 Decompressor #790

First revision of Panasonic RW2 version 8 Decompressor #790

kjyager commented Apr 3, 2025

kjyager Apr 3, 2025

kjyager Apr 3, 2025

kmilos Apr 3, 2025 •

edited

Loading

kjyager Apr 3, 2025

kmilos Apr 3, 2025 •

edited

Loading

kmilos Apr 3, 2025 •

edited

Loading

kmilos Apr 3, 2025 •

edited

Loading

kjyager Apr 3, 2025 •

edited

Loading

kmilos Apr 4, 2025

kjyager Apr 4, 2025

kmilos Apr 4, 2025

kjyager Apr 3, 2025

kjyager Apr 3, 2025

kjyager Apr 3, 2025

kjyager Apr 3, 2025

		// The set of templates and classes below define a specialized bit streamer which works mostly the same as
		// BitStreamerMSB, but which reverses the ordering of the bits within each byte.

First revision of Panasonic RW2 version 8 Decompressor #790

Are you sure you want to change the base?

First revision of Panasonic RW2 version 8 Decompressor #790

Conversation

kjyager commented Apr 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmilos Apr 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmilos Apr 3, 2025 • edited Loading

Choose a reason for hiding this comment

kmilos Apr 3, 2025 • edited Loading

Choose a reason for hiding this comment

kmilos Apr 3, 2025 • edited Loading

Choose a reason for hiding this comment

kjyager Apr 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmilos Apr 3, 2025 •

edited

Loading

kmilos Apr 3, 2025 •

edited

Loading

kmilos Apr 3, 2025 •

edited

Loading

kmilos Apr 3, 2025 •

edited

Loading

kjyager Apr 3, 2025 •

edited

Loading