Skip to content

Conversation

@LebedevRI
Copy link
Member

Based on #482 & #386

da-phil and others added 5 commits June 5, 2023 00:19
* Support large-resolution ARW from ILCE-7RM5
* Generalize sonyArrange in LJPEG arrangement to support 1x2

Co-authored-by: Artemis Tosini <me@artem.ist>
Recap: due to crop size roundup to multiples of tile size (512)
Sony lossless files exhibit black areas if not further cropped down
to actual pixel area, which is covered by the sensor.

Exif.SubImage1.0x7038 seems to reliably provide the true crop size,
whereas Exif.SubImage1.DefaultCropSize only crops to OOC jpeg size
losing some pixel area
* upstream/pr/482:
  Do not apply crop from cameras.xml and set crop origin to 0,0
  Boyscout: make sony exif tag naming consistent
  Use Exif.SubImage1.0x7038 field for Sony lossless compression crop
  ARW: Support new LJPEG compression on ILCE-7M4 and ILCE-7R5
@codecov
Copy link

codecov bot commented Jun 7, 2023

Codecov Report

Patch coverage: 65.21% and project coverage change: +20.68 🎉

Comparison is base (1ea45d9) 38.54% compared to head (836a697) 59.22%.

Additional details and impacted files
@@             Coverage Diff              @@
##           develop     #483       +/-   ##
============================================
+ Coverage    38.54%   59.22%   +20.68%     
============================================
  Files          643      232      -411     
  Lines        36905    13879    -23026     
  Branches      5073     1939     -3134     
============================================
- Hits         14224     8220     -6004     
+ Misses       22106     5524    -16582     
+ Partials       575      135      -440     
Flag Coverage Δ
benchmarks 8.26% <3.10%> (-0.29%) ⬇️
integration 47.64% <73.75%> (+0.27%) ⬆️
linux 57.05% <68.62%> (+0.19%) ⬆️
macOS 18.77% <0.67%> (-4.89%) ⬇️
rpu_u 47.64% <73.75%> (+0.27%) ⬆️
unittests 18.21% <3.10%> (-0.68%) ⬇️
windows ∅ <ø> (∅)

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

Impacted Files Coverage Δ
...zz/librawspeed/decompressors/LJpegDecompressor.cpp 0.00% <0.00%> (ø)
src/librawspeed/decoders/ArwDecoder.h 60.00% <ø> (ø)
src/utilities/rsbench/main.cpp 2.17% <0.00%> (+0.02%) ⬆️
src/librawspeed/decoders/ArwDecoder.cpp 48.97% <64.28%> (+2.42%) ⬆️
...rc/librawspeed/decompressors/LJpegDecompressor.cpp 60.00% <64.70%> (+5.38%) ⬆️
bench/librawspeed/io/BitStreamBenchmark.cpp 100.00% <100.00%> (ø)
src/librawspeed/adt/Point.h 72.83% <100.00%> (+0.33%) ⬆️
src/librawspeed/decompressors/LJpegDecoder.cpp 67.24% <100.00%> (+4.27%) ⬆️

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

Comment on lines +105 to +108
const iPoint2D MCUSize = !interleaveRows
? iPoint2D(frame.cps, 1)
: iPoint2D(frame.cps / 2, frame.cps / 2);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there still a plan to eliminate interleaveRows and keep generalizing this?

The reason I ask is, I don't see how this new MCUSize scheme can support the Blackmagic case, which would need something like iPoint2D(frame.cps / 2, frame.cps * 2) with frame.cps / 2 having to be 0.5!?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there still a plan to eliminate interleaveRows and keep generalizing this?

LJpegDecoder should not take interleaveRows, so i guess this needs to be generalized further, yes.

The reason I ask is, I don't see how this new MCUSize scheme can support the Blackmagic case, which would need something like iPoint2D(frame.cps / 2, frame.cps * 2) with frame.cps / 2 having to be 0.5!?

No idea. That is a future me problem.

Copy link
Collaborator

@kmilos kmilos Jun 8, 2023

Choose a reason for hiding this comment

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

That is a future me problem.

Might warrant a thought though, so you don't have to potentially tear down what you're building up now? Not that there is anything wrong with that if that process works for you ;)

@LebedevRI
Copy link
Member Author

Ok, i can't be bothered.

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.

3 participants