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.

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

Jump to bottom

Adding OM-5 camera (by OM Digital Solutions) #399

Closed
wants to merge 0 commits into from
Closed

Adding OM-5 camera (by OM Digital Solutions) #399

wants to merge 0 commits into from

Conversation

DE-cr
Copy link
Contributor

@DE-cr DE-cr commented Nov 16, 2022

notes:

kmilos
kmilos reviewed Nov 16, 2022
View reviewed changes
data/cameras.xml Outdated
@@ -7387,6 +7387,18 @@
</ColorMatrix>
</ColorMatrices>
</Camera>
<Camera make="OM Digital Solutions" model="OM-5">
Copy link
Contributor

@kmilos kmilos Nov 16, 2022 •
edited

Choose a reason for hiding this comment

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

camera metrics copied from Olympus E-M1MarkIII, from which the OM-5 inherits its sensor

Add a comment this is copy of Olympus E-M5 Mark III data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, it's a copy of Olympus E-M1MarkIII data.
I didn't notice a way to add comments in the xml, sorry.

Copy link
Contributor

@kmilos kmilos Nov 16, 2022 •
edited

Choose a reason for hiding this comment

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

Actually, it's a copy of Olympus E-M1MarkIII data.

I thought the DP Review said this was basically a rebranded E-M5MarkIII... In any case, it doesn't matter as they both seem to have the same sensor. Black level is ISO sensitive and doesn't matter much here (it is 256 on the DPR ISO 80 sample), and is read dynamically from metadata anyway for ORFs.

I didn't notice a way to add comments in the xml, sorry.

There are quite a few examples of the "copied from" in the file already...

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'm sorry, but I cannot find such comments in cameras.xml.
(I have seen them in *.cpp, though, but that's not part of the change here.)

E-M1MarkIII seems to have inherited its sensor from the E-M5MarkIII, but there may (or may not) have been some (minor) improvements along the way. I've read in OM-5 reviews that image quality is improved over E-M5MarkIII.

Copy link
Contributor

@kmilos kmilos Nov 16, 2022 •
edited

Choose a reason for hiding this comment

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

I'm sorry, but I cannot find such comments in cameras.xml.

True! Looks like they've been blown away by the script re-generating this file, but I remember seeing them in the (distant) past... Never mind then, as they will probably again get removed by a script later anyway.

@kmilos
Copy link
Contributor

kmilos commented Nov 16, 2022

Probably needs samples in RPU before being included, see #357 (comment)

@DE-cr
Copy link
Contributor Author

DE-cr commented Nov 16, 2022

I've just uploaded a full set of sample raw files for OM-5 to https://raw.pixls.us

@DE-cr
Copy link
Contributor Author

DE-cr commented Nov 19, 2022

I've just uploaded a full set of sample raw files for OM-5 to https://raw.pixls.us

...and I've since used this pr in my custom version of darktable 4.0.1 for plenty of raw files from my OM-5, both regular 20 MP pictures, 50 MP handheld hi-res and 80 MP tripos hi-res. Works like a charm (as expected).

@kmilos kmilos mentioned this pull request Dec 5, 2022
Closed
@DE-cr DE-cr mentioned this pull request Dec 15, 2022
Closed
@LebedevRI LebedevRI self-assigned this Jan 29, 2023
@LebedevRI
Copy link
Member

I accidentally posted this in the PR it seems. Moving here from #357 (comment):

So i was going to merge this, but i'm not going to.
I'm not sure just how exactly else we can reword the message on https://raw.pixls.us/.
NO COLOR CHARTS.

Please contribute the same 5-sample set that is currently available there for that camera,
but of something else, like daylight landscape. Then i'll merge this.

@LebedevRI
Copy link
Member

@DE-cr thank you for your contribution!

@DE-cr
Copy link
Contributor Author

DE-cr commented Jan 29, 2023

Sorry, I was too eager to get support for my new camera in darktable's 2022 "Christmas" release that I did the test shots immediately after receiving the OM-5, without carefully reading the instructions. I'll upload a different set.

@DE-cr
Copy link
Contributor Author

DE-cr commented Jan 29, 2023

@LebedevRI I've now uploaded a new set of raw files for the OM-5 that don't show a color target. Please have the old OM-5 sample files (upload from 2022-11-16) removed from pixls.us

@LebedevRI
Copy link
Member

Thanks!
Could you please also rebase this, and ideally squash all commits into one?

@LebedevRI LebedevRI closed this Jan 29, 2023
LebedevRI pushed a commit that referenced this pull request Jan 29, 2023
@DE-cr @LebedevRI
Adding OM-5 camera (by OM Digital Solutions)
18f1e18 
notes:
- this adds model="OM-5" to #357, which clears the path in adding make="OM Digital Solutions"
- camera metrics copied from Olympus E-M1MarkIII, from which the OM-5 inherits its sensor

Fixes #399
@LebedevRI
Copy link
Member

I should have just waited i guess. Please, it's best to use a separate branch for pull requests,
it gets unnessesairly messy otherwise. I've merged this locally in 18f1e18.

@DE-cr Thank you for the contribution!

@DE-cr
Copy link
Contributor Author

DE-cr commented Jan 29, 2023

Thanks! Could you please also rebase this, and ideally squash all commits into one?

I'm sorry, should I still do anything here, @LebedevRI ?

@LebedevRI LebedevRI mentioned this pull request Feb 4, 2023
Closed
@LebedevRI
Copy link
Member

Thanks! Could you please also rebase this, and ideally squash all commits into one?

I'm sorry

That was my fault.

should I still do anything here, @LebedevRI ?

No. In general, after things are merged, it becomes my problem,
and original contributor can't really be asked for anything more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kmilos kmilos kmilos left review comments

@LebedevRI LebedevRI Awaiting requested review from LebedevRI

Assignees

@LebedevRI LebedevRI

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@DE-cr @kmilos @LebedevRI