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.

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

Fix eos imageformat #1031

Merged
merged 3 commits into from
Sep 27, 2024
Merged

Fix eos imageformat #1031

merged 3 commits into from
Sep 27, 2024

Conversation

axxel
Copy link
Contributor

@axxel axxel commented Sep 27, 2024

TLDR: new imageformat2 widget to later replace current one with consistent and complete support for all formats on all EOS cameras. Details, see commit messages. Example from my 5Ds:

Label: Image Format 2                                                          
Readonly: 0
Type: RADIO
Current: mRAW + S3
Choice: 0 L
Choice: 1 cL
Choice: 2 M1
Choice: 3 cM1
Choice: 4 M2
Choice: 5 cM2
Choice: 6 S1
Choice: 7 cS1
Choice: 8 S2
Choice: 9 S3
Choice: 10 RAW + L
Choice: 11 RAW + cL
Choice: 12 RAW + M1
Choice: 13 RAW + cM1
Choice: 14 RAW + M2
Choice: 15 RAW + cM2
Choice: 16 RAW + S1
Choice: 17 RAW + cS1
Choice: 18 RAW + S2
Choice: 19 RAW + S3
Choice: 20 mRAW + L
Choice: 21 mRAW + cL
Choice: 22 mRAW + M1
Choice: 23 mRAW + cM1
Choice: 24 mRAW + M2
Choice: 25 mRAW + cM2
Choice: 26 mRAW + S1
Choice: 27 mRAW + cS1
Choice: 28 mRAW + S2
Choice: 29 mRAW + S3
Choice: 30 sRAW + L
Choice: 31 sRAW + cL
Choice: 32 sRAW + M1
Choice: 33 sRAW + cM1
Choice: 34 sRAW + M2
Choice: 35 sRAW + cM2
Choice: 36 sRAW + S1
Choice: 37 sRAW + cS1
Choice: 38 sRAW + S2
Choice: 39 sRAW + S3
Choice: 40 RAW
Choice: 41 mRAW
Choice: 42 sRAW
END

Instead of :

Label: Image Format                                                            
Readonly: 0
Type: RADIO
Current: RAW + Large Fine JPEG
Choice: 0 Large Fine JPEG
Choice: 1 Large Normal JPEG
Choice: 2 Unknown value 53ff
Choice: 3 Unknown value 52ff
Choice: 4 Unknown value 63ff
Choice: 5 Unknown value 62ff
Choice: 6 Small Fine JPEG
Choice: 7 Small Normal JPEG
Choice: 8 Smaller JPEG
Choice: 9 Tiny JPEG
Choice: 10 RAW + Large Fine JPEG
Choice: 11 RAW + Large Normal JPEG
Choice: 12 Unknown value 0c53
Choice: 13 Unknown value 0c52
Choice: 14 Unknown value 0c63
Choice: 15 Unknown value 0c62
[...]

@ben5516 and @karlkopf, could you test if gphoto2 --get-config imageformat2 produces something meaningful with your 1DXm2 / 1DXm3 / R5m2 cameras?

We use hex format for the same information in other parts of the log file.
This brings my original ptp_unpack_EOS_ImageFormat() documentation
up-to-date with new M1/M2 and new 0/1 compression info added with 5Ds,
1DXm2 and R5m2.

It also fixes the problem that led to a double entry of e.g. `RAW` in the
imageformat config output visible in gphoto#1028 and also in the 1DX sample here:
https://github.com/gphoto/libgphoto2/blob/5b7f7c168dd3e86f43b590a7456883e1dec11709/camlibs/ptp2/cameras/canon-eos-1dx.txt#L255-L269
The current `imageformat` with it's table lookup for every RAW+JPEG
combination became cumbersome to maintain and outdated. This leads to
e.g. lost of unsupported JPEG variants for e.g. the 5Ds, R5m2, 1DX.

The used naming also became inconsistent with the addition of the 1DXm2
labels in gphoto@1402f88

This patch adds an (incompatible) new imageformat2 widget as a temporary
test to then hopefully replace the existing imageformat. I expect this new
implementation to fix all "Unknown value" occurrences of all currently
available EOS cameras.

I chose to do away with the verbose and arbitrary labels like
"Smaller Fine JPEG" and instead tried to mimic the strings that are
actually used by the cameras, like 'S2', etc. I added a 'c' for the
higher compression versions, along the lines of 'cRAW'. The 'c' could
also stand for 'coarse' to describe the blocky curve that is used by the
Canons to distinguish between lower and higher Jpeg quality.

Here is some sample output from the 5Ds:
Choice: 5 cM2
Choice: 6 S1
Choice: 7 cS1
Choice: 8 S2
Choice: 9 S3
Choice: 10 RAW + L
Choice: 11 RAW + cL
Choice: 12 RAW + M1
Choice: 13 RAW + cM1
@msmeissn msmeissn merged commit 042c4f3 into gphoto:master Sep 27, 2024
5 checks passed
@axxel
Copy link
Contributor Author

axxel commented Sep 27, 2024

@msmeissn Thanks for merging this. Do you have an opinion on the idea of replacing imageformat with imageformat2 as is, meaning a user facing change of the enumerated strings Large Fine JPEG -> L or RAW + Small Normal JPEG -> RAW + cS?

@axxel axxel deleted the fix-eos-imageformat branch September 27, 2024 08:28
@axxel
Copy link
Contributor Author

axxel commented Sep 27, 2024

Thanks to @karlkopf we have a confirmation that the code works for the R5m2.

@msmeissn
Copy link
Contributor

@msmeissn Thanks for merging this. Do you have an opinion on the idea of replacing imageformat with imageformat2 as is, meaning a user facing change of the enumerated strings Large Fine JPEG -> L or RAW + Small Normal JPEG -> RAW + cS?

I think we can do that. I am currently not aware of tooling parsing these directly.

@Sija
Copy link
Contributor

Sija commented Sep 29, 2024

I wouldn't like having abbreviated settings since it makes it difficult to understand what is what.

@axxel
Copy link
Contributor Author

axxel commented Sep 29, 2024

I wouldn't like having abbreviated settings since it makes it difficult to understand what is what.

I understand your sentiment but my point is that those new labels are in fact not abbreviations of existing settings but rather, the old ones are "invented" terms that did not faithfully mirror the actual settings on the devices. I did a little digging in my memory and the commit history and found that I originally named them like this: "L/Fine" when I implemented the imageformat setting of the EOS cameras (so close to what I proposed here), then later tried to improve the situation by renaming that to "Large Fine JPEG" with the idea to make the used terminology consistent with settings of other manufacturers. Such that you could potentially have the same client code working across multiple vendors. While that might work for the trivial case "Large Fine JPEG", it falls apart with more vendor specific options like "S2". The latter made me invent the term "Smaller JPEG" which is just arbitrary and comes at the price that the name has no connection to the names used by Canon anymore. Someone else didn't like that either.

Bottom line: today I think my intention to have those verbose names consistent with other settings failed and might have been doomed to do so, e.g. other settings added later are similar but then still inconsistent (see +RAW in this case). So I now believe it is better to stick to what the vendors actually use and definitively keep the terminology consistent within one vendor setting, and "Tiny JPEG" definitively failed there.

We could argue over whether L should maybe rather read L JPEG but then cRAW would arguably have to be called cRAW RAW which looks silly to me. I definitively dislike the term "Normal" now, which has no meaning whatsoever. And the very latest models like the R5m2 don't distinguish between "Normal" and "Fine" anymore, there is only one "L" option and you can customize the JPEG compression level in a different setting.

@Sija
Copy link
Contributor

Sija commented Oct 12, 2024

Thanks for going down the memory lane with me, much appreciated.

Looking at it from the client-side perspective, making these options/values vendor-specific, forces each developer to create the same spaghetti code for a generic way of accessing them programatically.

Why not unify them across the drivers and document the chosen format?

@axxel
Copy link
Contributor Author

axxel commented Oct 13, 2024

Why not unify them across the drivers and document the chosen format?

As I elaborated above, that was exactly my intention 10 years ago but now came to the conclusion that this effort failed and never had a chance. Thy are already vendor specific in two aspects:

  1. the number of widgets itself is already vendor specific: some have quality and size and RAW vs. JPEG in one widget, others separate that more or less.
  2. the entries of the widgets are different and really have to be as soon as you leave the standard RAW / L / S route.

I could imagine a kind of "meta" widget that contains a generic subset of sizes and basically contains the "spagetti" code that maps those generic terms to vendor specific ones. Do you have a suggestion how this basic and generic set of formats/sizes could look like?

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