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

Update canon tags #1628

Closed
wants to merge 650 commits into from
Closed

Update canon tags #1628

wants to merge 650 commits into from

Conversation

alexvanderberkel
Copy link
Member

I think we still need to work a bit on the new Canon tags. For instance, we have these tags identified
image

However, there not nicely being displayed at the moment if you compare it with information from the exiftool / page.

image

@kmilos I think we need to add somewhere a prettier printer, isn't that right?

@kmilos
Copy link
Collaborator

kmilos commented May 11, 2021

@alexvanderberkel Sure, but that's easy as pie (if there is no additional munging of data required), just add your dictionary (e.g. canonHDR) at the top of canonmn_int.cpp and use EXV_PRINT_TAG(canonHDR) as the print function. Follow canonPictureStyle for an example.

@kmilos
Copy link
Collaborator

kmilos commented May 11, 2021

Ah, sorry, I just now got HDRInfo is one tag with an array of encoded values, so not the straightforward case...

src/canonmn_int.cpp Outdated Show resolved Hide resolved
@@ -1046,13 +1190,24 @@ namespace Exiv2 {
{ Tag::root, fujiId, exifId, 0x927c },
{ Tag::root, canonId, exifId, 0x927c },
{ Tag::root, canonCsId, canonId, 0x0001 },
{ Tag::root, canonSiId, canonId, 0x0004 },
{ Tag::root, canonSiId, canonId, 0x0004 },
{ Tag::root, canonAf2Id, canonId, 0x0026 },
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that this works, because the AF field of canon maker notes is handled in a special way.
See line 1816 where we define a special TiffDecoder for this element.

That's why I think that we can actually remove most (maybe all) the AF stuff from canonmn_int.{hpp,cpp}

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I see the following:

Exif.CanonAf2Id.AFInfoSize SLong 1 1216
Exif.CanonAf2Id.AFAreaMode SLong 1 (65679)
Exif.CanonAf2Id.AFNumPoints SLong 1 293608000
Exif.CanonAf2Id.AFValidPoints SLong 1 293608000
Exif.CanonAf2Id.AFCanonImageWidth SLong 1 468
Exif.CanonAf2Id.AFCanonImageHeight SLong 1 0
Exif.CanonAf2Id.AFImageWidth SLong 1 0
Exif.CanonAf2Id.AFImageHeight SLong 1 0
Exif.CanonAf2Id.AFAreaWidths SLong 1 0
Exif.CanonAf2Id.AFAreaHeights SLong 1 0
Exif.CanonAf2Id.AFXPositions SLong 1 0
Exif.CanonAf2Id.AFYPositions SLong 1 0
Exif.CanonAf2Id.AFPointsInFocus SLong 1 0
Exif.CanonAf2Id.AFPointsSelected SLong 1 0
Exif.CanonAf2Id.AFPointsUnusable SLong 1 0
Exif.CanonAf2Id.AFFineRotation SLong 1 0

And:

Exif.CanonAfC.AFConfigTool SLong 1 0
Exif.CanonAfC.AFTrackingSensitivity SLong 1 2
Exif.CanonAfC.AFAccelDecelTracking SLong 1 2
Exif.CanonAfC.AFPointSwitching SLong 1 0
Exif.CanonAfC.AIServoFirstImage SLong 1 Equal Priority
Exif.CanonAfC.AIServoSecondImage SLong 1 Equal Priority
Exif.CanonAfC.USMLensElectronicMF SLong 1 Disable in AF Mode
Exif.CanonAfC.AFAssistBeam SLong 1 Disable
Exif.CanonAfC.OneShotAFRelease SLong 1 Release Priortiy
Exif.CanonAfC.AutoAFPointSelEOSiTRAF SLong 1 Enable
Exif.CanonAfC.LensDriveWhenAFImpossible SLong 1 Continue Focus Search
Exif.CanonAfC.SelectAFAreaSelectionMode SLong 1 (22359)
Exif.CanonAfC.AFAreaSelectionMethod SLong 1 M-Fn Button
Exif.CanonAfC.OrientationLinkedAF SLong 1 Same for Vert/Horiz Points
Exif.CanonAfC.ManualAFPointSelPattern SLong 1 Stops at AF Area Edges
Exif.CanonAfC.AFPointDisplayDuringFocus SLong 1 Selected (constant)
Exif.CanonAfC.VFDisplayIllumination SLong 1 Auto
Exif.CanonAfC.AFStatusViewfinder SLong 1 Auto
Exif.CanonAfC.InitialAFPointInServo SLong 1 Initial AF Point Selected

Copy link
Member Author

Choose a reason for hiding this comment

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

Exif.CanonAfC.SelectAFAreaSelectionMode SLong 1 (22359) --> This tag needs some more work though

Copy link
Member Author

Choose a reason for hiding this comment

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

Exif.CanonAf2Id.AFAreaMode SLong 1 (65679) --> This could also need some digging

@alexvanderberkel alexvanderberkel self-assigned this May 20, 2021
@alexvanderberkel alexvanderberkel added this to the v1.00 milestone May 26, 2021
kevinbackhouse and others added 20 commits August 30, 2021 12:41
…l?id=37889

Avoid reading 1 byte off the end when the string does not contain a '\0' byte.
Avoid reading 1 byte off the end when the string does not contain a '\0' byte
Call Metadatum::print() to increase fuzzing coverage.
Update the `COMMANDS` and `EXIV2 GROUPS, TYPES AND VALUES` sections,
including any related sections.

Changes:
+ Add new examples and update text
+ Fix spelling errors
Original manpage file text is replaced with a note pointing to the
Exiv2 website.
Changes:
+ Update BMFF types in FILE TYPES section
+ Add text in FILE TYPES explaining formats
+ Change IPTC datasets and XMP properties to IPTC and XMP tags
+ Fix text and formatting
@piponazo
Copy link
Collaborator

piponazo commented Feb 8, 2022

Hi @alexvanderberkel , there seems to be something really wrong with the status of your branch (it says it has 650 commits 🤯 ) . I would recommend you to either rebase your commits in top of main or start a new branch from scratch and reapply your commits.

@alexvanderberkel
Copy link
Member Author

Hi @alexvanderberkel , there seems to be something really wrong with the status of your branch (it says it has 650 commits exploding_head ) . I would recommend you to either rebase your commits in top of main or start a new branch from scratch and reapply your commits.

did a rebase lately. I think it based on the fact that there was quite some work by a lot of other people over the month. I had to catch up with a lot things.

@clanmills
Copy link
Collaborator

clanmills commented Feb 9, 2022

You have two errors being detected by the test suite in github/981 and redmine/445.

github/981

This is easy. You are now identifying three unknown tags: OpticalZoomCode (0x10), AEBBracketValue (0x11) and ControlMode (0x12). Fix is easy and in the patch below.

redmine/445

Good and Horrible News

Good: You have inflicted damage on the the test file. It runs exiv2 6 times. Somehow, you've lost 5 of the outputs. I've restored the "old" test and now it runs.

Horrible: I can't remember what redmine/445 is about, but it's got my finger prints all over it. It concerns Canon AFInfo and should report metadata with the keys such as Exif.Canon.AFInfoSize. All of that metadata has disappeared. I'll have to investigate what you've changed to cause this.

I recommend that you submit the patch as a step in the right direction. The errors should drop to 1 (plus the bitch that bugfixTests has failed.

Patch:

diff --git a/tests/bugfixes/github/test_issue_981.py b/tests/bugfixes/github/test_issue_981.py
index 6cc0e622..f6455588 100644
--- a/tests/bugfixes/github/test_issue_981.py
+++ b/tests/bugfixes/github/test_issue_981.py
@@ -32,7 +32,7 @@ Exif.Canon.AFYPositions                      SShort      9  0 321 -321 603 0 -60
 Exif.Canon.AFPointsInFocus                   Short       1  4
 Exif.Canon.AFPointsSelected                  Short       1  8
 Exif.Canon.AFPointsUnusable                  Short       1  (none)
-Exif.Canon.AFInfo                            Short     273  546 2 63 61 6720 4480 6720 4480 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 0 0 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 0 0 65200 64790 64435 64099 63764 336 0 65200 64099 63764 1772 1437 1101 746 336 0 1437 1101 746 336 0 65200 64790 64435 336 0 65200 64790 64435 64099 63764 1772 64790 64435 64099 63764 1772 1437 1101 746 63764 1772 1437 1101 746 336 0 65200 1101 746 336 0 65200 64790 64435 64099 336 0 65200 1772 1437 0 0 547 625 625 625 625 821 821 821 308 308 625 625 625 625 547 547 308 308 308 274 274 274 308 308 0 0 0 0 0 0 0 308 65228 65228 65228 65228 0 0 0 0 64911 65228 65228 65228 65228 65262 65262 65262 64911 64911 64989 64989 64989 64911 64911 64911 64715 64715 64715 64911 64911 0 0 0 512 0 0 0 512 0 0 0 0 0 0 65535
+""" , """Exif.Canon.AFInfo                            Short     273  546 2 63 61 6720 4480 6720 4480 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 0 0 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 218 0 0 65200 64790 64435 64099 63764 336 0 65200 64099 63764 1772 1437 1101 746 336 0 1437 1101 746 336 0 65200 64790 64435 336 0 65200 64790 64435 64099 63764 1772 64790 64435 64099 63764 1772 1437 1101 746 63764 1772 1437 1101 746 336 0 65200 1101 746 336 0 65200 64790 64435 64099 336 0 65200 1772 1437 0 0 547 625 625 625 625 821 821 821 308 308 625 625 625 625 547 547 308 308 308 274 274 274 308 308 0 0 0 0 0 0 0 308 65228 65228 65228 65228 0 0 0 0 64911 65228 65228 65228 65228 65262 65262 65262 64911 64911 64989 64989 64989 64911 64911 64911 64715 64715 64715 64911 64911 0 0 0 512 0 0 0 512 0 0 0 0 0 0 65535
 Exif.Canon.AFInfoSize                        SShort      1  546
 Exif.Canon.AFAreaMode                        SShort      1  Single-point AF
 Exif.Canon.AFNumPoints                       SShort      1  63
@@ -50,18 +50,18 @@ Exif.Canon.AFPointsSelected                  Short       4  25
 Exif.Canon.AFPointsUnusable                  Short       4  (none)
 Exif.Canon.AFMicroAdj                        Long       11  44 2 0 10 4294967295 0 10 0 10 0 10
 Exif.Canon.AFConfig                          Long       20  80 4 0 0 1 0 0 0 0 1 0 1 32639 1 0 1 0 0 0 2
-Exif.Canon.AFNumPoints                 SShort      1  63
-Exif.Canon.AFValidPoints               SShort      1  61
-Exif.Canon.AFPointsInFocus             Short       4  0 560 57344 0
-Exif.Canon.AFPointsSelected            Short       4  0 1848 57344 0
-Exif.Canon.AFPointsUnusable            Short       4  0 0 0 0
-Exif.Canon.AFNumPoints                       SShort      1  63
+""","""0x2602 Canon        AFNumPoints                 SShort      1  63
+0x2603 Canon        AFValidPoints               SShort      1  61
+0x260c Canon        AFPointsInFocus             Short       4  0 560 57344 0
+0x260d Canon        AFPointsSelected            Short       4  0 1848 57344 0
+0x260e Canon        AFPointsUnusable            Short       4  0 0 0 0
+""","""Exif.Canon.AFNumPoints                       SShort      1  63
 Exif.Canon.AFValidPoints                     SShort      1  61
 Exif.Canon.AFPointsInFocus                   Short       4  20,21,25,45,46,47
 Exif.Canon.AFPointsSelected                  Short       4  19,20,21,24,25,26,45,46,47
 Exif.Canon.AFPointsUnusable                  Short       4  (none)
-Exif.Canon.AFPointsUnusable            Short       4  3608 49152 792 6272
-Exif.Canon.AFPointsUnusable                  Short       4  3,4,9,10,11,30,31,35,36,40,41,55,59,60
+""","""0x260e Canon        AFPointsUnusable            Short       4  3608 49152 792 6272
+""","""Exif.Canon.AFPointsUnusable                  Short       4  3,4,9,10,11,30,31,35,36,40,41,55,59,60
 """
 ]
     stderr = [""]*len(commands)
diff --git a/tests/bugfixes/redmine/test_issue_445.py b/tests/bugfixes/redmine/test_issue_445.py
index 4b5327c3..4ca88b0c 100644
--- a/tests/bugfixes/redmine/test_issue_445.py
+++ b/tests/bugfixes/redmine/test_issue_445.py
@@ -99,15 +99,15 @@ Exif.CanonSi.ExposureCompensation            Short       1  0
 Exif.CanonSi.WhiteBalance                    Short       1  Auto
 Exif.CanonSi.SlowShutter                     Short       1  Off
 Exif.CanonSi.Sequence                        Short       1  0
-Exif.CanonSi.0x000a                          Short       1  6
+Exif.CanonSi.OpticalZoomCode                 Short       1  6
 Exif.CanonSi.0x000b                          Short       1  0
 Exif.CanonSi.CameraTemperature               Short       1  --
 Exif.CanonSi.FlashGuideNumber                Short       1  0
 Exif.CanonSi.AFPointUsed                     Short       1  3 focus points; center used
 Exif.CanonSi.FlashBias                       Short       1  0 EV
 Exif.CanonSi.AutoExposureBracketing          Short       1  Off
-Exif.CanonSi.0x0011                          Short       1  0
-Exif.CanonSi.0x0012                          Short       1  1
+Exif.CanonSi.AEBBracketValue                 Short       1  0
+Exif.CanonSi.ControlMode                     Short       1  1
 Exif.CanonSi.SubjectDistance                 Short       1  7.82 m
 Exif.CanonSi.0x0014                          Short       1  0
 Exif.CanonSi.ApertureValue                   Short       1  F5

@clanmills
Copy link
Collaborator

@piponazo Can I ask you to use your awesome git skills to rebase this project and update this PR. GitHub thinks 368 files have been changed. That's not right. 360+ other files have changes in the code-base since this PR was created last May.

I'd like to understand what has been changed by Alex, and then fix the disappearance of all Exif.Canon.AFInfo tags.

The Canon.AFInfo decoder is

{ "*",         0x0026, canonId,   &TiffDecoder::decodeCanonAFInfo,  0 /* Exiv2.Canon.AFInfo is read-only */ },`

Christoph has added something to the state table:

        { Tag::root, canonAf2Id,       canonId,          0x0026    },

I think that's is involved in causing all the Exif.Canon.AFInfo metadata to disappear. A clean and simple diff be of great value.

@piponazo
Copy link
Collaborator

piponazo commented Feb 9, 2022

@piponazo Can I ask you to use your awesome git skills to rebase this project and update this PR. GitHub thinks 368 files have been changed. That's not right. 360+ other files have changes in the code-base since this PR was created last May.

Sure, I will give it a try. I'll create a second branch update_canon_tags_2 where to make the experiment. I'll let you know if I succeed 🤞

@clanmills
Copy link
Collaborator

clanmills commented Feb 9, 2022

@piponazo You are a splendid fellow, @piponazo. Thank You. I suspect we'll open a new PR to deal with the issue that Alex is trying to solve.

@hassec has made a very clever observation about modifying the state table to deal with 0x0026. It's possible that my code in #981 can be eliminated. However that's different from the issue that Alex set out to solve.

@piponazo
Copy link
Collaborator

piponazo commented Feb 9, 2022

@clanmills @alexvanderberkel I could successfully move all the commits from @alexvanderberkel to the branch update_canon_tags_2. There were some troubles during the process, so please double check that all the changes are there and I did not forget to cherry-pick something.

You can see that the latest 3 commits are from my side to fix some compilation issues and tests output expectations:
https://github.com/Exiv2/exiv2/commits/update_canon_tags_2

Compilation seems to be fine, but there are some tests failing:
https://github.com/Exiv2/exiv2/actions/runs/1820708617

@clanmills
Copy link
Collaborator

@piponazo Thanks for dealing with this, Luis. I'll look at it tomorrow (Thursday).

@clanmills
Copy link
Collaborator

@alexvanderberkel Even with Luis' great work, I have a problem building both your branch and Luis new branch. The problem is caused on macOS by libiconv. This was fixed in May last year I can't remember how.

There's a saying in English "When you are in a hole, stop digging". We need to back up and open a bug report which precisely explains the issue. I realise it has something to do with Canon tags and ExifTool compatibility. Can you document the issue? Let's ask @kmilos and/or @hassec to deal with it.

Fixing this is is not urgent as the issue always has been in the code base.

@alexvanderberkel
Copy link
Member Author

@piponazo thanks.

@clanmills
Copy link
Collaborator

The linking issue on the MacMini is caused by brew or macports having installed libiconv in /opt/local. cmake correctly generated the build to use libiconv in the macOS/SDK. The linker finds the wrong library and fails as follows:

Undefined symbols for architecture x86_64:
  "_libiconv", referenced from:
      Exiv2::convertStringCharset(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&, char const*, char const*) in convert.cpp.o

My fix was to remove the library from /opt/local. It's likely that there's poison in my environment that causes this.

env | grep -i lib
PERL5LIB=/Users/rmills/bin
PATH=~/Library/Python/3.8/bin:/usr/local/opt/gettext/bin:.:/Users/rmills/bin:/Users/rmills/bin/macosx:/usr/local/bin:/usr/bin:/usr/sbin:/bin:/sbin:/usr/X11R6/bin:~/Library/Python/3.8/bi:/Library/Developer/Toolchains/swift-latest.xctoolchain/usr/bin:/opt/local/bin:/opt/local/sbin:/opt/pkgconfig/bin
DYLD_LIBRARY_PATH=/Users/rmills/gnu/boost/boost_1_66_0
BOOST_LIBRARYDIR=/Users/rmills/gnu/boost/boost_1_66_0/libs
PYTHONPATH=/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages:
PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/lib/pkgconfig:/opt/local/lib/pkgconfig
620 rmills@rmillsmm-local:~/gnu/github/exiv2/update_canon_tags_2/build $ 

@clanmills clanmills mentioned this pull request Feb 10, 2022
@clanmills
Copy link
Collaborator

@alexvanderberkel. Luis has created a new branch update_canon_tags_2 and I've fixed the tests in that branch. Test 981 is failing. Let's discuss it one-to-one on WhatsApp. #2087

@clanmills
Copy link
Collaborator

clanmills commented Feb 10, 2022

I'm going to close this (without deleting the branch at the moment). Conversation continues in #2087.

@piponazo Thank You very much for your help with this. A little easier to understand the changes in 24 files. I'll update README.md and CONTRIBITING.md at the weekend. I'll make sure the documented test procedure is correct and I'll remove all mention of the make utility.

@clanmills clanmills closed this Feb 10, 2022
@clanmills
Copy link
Collaborator

I'm going to delete this branch. We don't need it. #2087 deals with this.

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.