-
Notifications
You must be signed in to change notification settings - Fork 115
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
Phase One P25+ support #704
Conversation
ed58ede
to
3d703cf
Compare
@kmilos thank you! |
That's still unfortunate, this was not too far off (maybe 2px on each side) from the vendor embedded crop... |
By this, do you mean the one in your diff, or the correct one? |
The one in my original patch - vendor has 26,21 for top left, I started w/ 26,20 (tightest possible). So 26,22 or 28,22 should've worked in theory (w/ -12,-12 bottom right instead of my tight -10,-10)... Could have something to do w/ native portrait readout of the sensor? Border artifacts still there if you bypass the orientation module? Otherwise I guess we could still open a tracking issue for dt demosaic (or whatever module in the pipeline)? |
At least here demosaic happens first, so that can't be it.
Since tighter crop fixes it, and rawprepare actually resizes the image and drops cropped-off pixels, |
And I don't really see why supplying all the valid pixels wouldn't work either, unless there is something funny going on w/ border processing somewhere... (E.g. related to stride, vector length etc.) @jenshannoschwalm again, sorry |
But how do we know that all the pixels as specified by that vendor tag are the valid pixels?
But again, if there was a bug, i don't really see why tightening the crop would magically fix it. |
Dump from
As mentioned, changes stride? |
In all 8 demosaic algos, including passthrough? |
Could also be elsewhere in the pipeline as I said, not necessarily demosaic... Some underlying tiling/LOD logic? Anyhow, just find it weird this keeps popping back. If you're happy w/ the workaround of tweaking the crop, we'll just keep doing that. |
I will have a close look after this has landed in dt master. |
Resolves darktable-org/darktable#16539