Bug #1202

Exif.CanonCs.FocusContinuous = 8 = Manual

Added by Sridhar Boovaraghavan 11 months ago. Updated 11 months ago.

Status:ClosedStart date:11 Aug 2016
Priority:NormalDue date:
Assignee:Robin Mills% Done:

100%

Category:makernoteEstimated time:3.00 hours
Target version:0.26

Description

Please change it to signed short (follows your documentation, see http://www.exiv2.org/tags-canon.html) and it is being interpreted as an unsignedShort in the code. 65535 is the unsigned short version of -1 (signedShort).

For reference, exiftool reports -1.

All indications are that this is a bug in exiv2. It not being raised in 8 years possibly just means that no one was interested in that tag till now.

As to what -1 actually means, that's a different question. I was proposing that we interpret -1 to mean Single (i.e. the same as zero).

I also propose that you add the value 8 to count as Manual (following exiftool, please see http://www.sno.phy.queensu.ca/~phil/exiftool/TagNames/Canon.html). Worth noticing that it is treated as signed short.

For testing purposes, one could use exiv2's test files itself. Try exiv2\test\data\exiv2-test.out(Line #2697). See that it reports (65535) whereas it should be a -1.

Regards,
Sridhar


Related issues

Related to Exiv2 - Bug #1203: Exif.CanonCs.FocusContinuous reported as Short Assigned 11 Aug 2016

Associated revisions

Revision 4365
Added by Robin Mills 11 months ago

#1202 Adding "Manual" Exif.CanonCs.FocusContinuous

Revision 4370
Added by Robin Mills 11 months ago

#1202 Update test suite. The test jpg was originally test/tmp/20030925_201850.jpg

History

#1 Updated by Robin Mills 11 months ago

  • Category set to makernote
  • Status changed from New to Resolved
  • Assignee set to Robin Mills
  • Priority changed from High to Normal
  • Target version set to 0.26
  • % Done changed from 0 to 100
  • Estimated time set to 3.00

I've looked carefully at this. I'm not convinced that there is a bug here. Here's my point of view. I've modified the code (change below) and the output is now as you request:

$ exiv2 -pa -K Exif.CanonCs.FocusContinuous ~gnu/exiv2/trunk/test/tmp/20030925_201850.jpg
Exif.CanonCs.FocusContinuous                 Short       1  (-1)
818 rmills@rmillsmbp:~/gnu/exiv2/trunk $ 
However you will observe that the type of the data in "Short" and not "SShort". The fix requires me to introduce a bug in the code in tags_int.hpp#226
    template <int N, const TagDetails (&array)[N]>
    std::ostream& printTag(std::ostream& os, const Value& value, const ExifData*)
    {
        const TagDetails* td = find(array, value.toLong());
        if (td) {
            os << exvGettext(td->label_);
        }
        else {
            if ( value.typeId() == unsignedShort ) {
                signed short v = (signed short) value.toLong();

                os << "(" << v << ")";
            } else {
                os << "(" << value << ")";
            }
        }
        return os;
    }
So the question is "Why is value being delivered as Short and not an SShort?". The tag is defined as signedShort in canonmn.cpp#1134.
TagInfo(0x0020, "FocusContinuous" 
, N_("Focus Continuous"), N_("Focus continuous setting")
, canonCsId, makerTags, signedShort, 1
, EXV_PRINT_TAG(canonCsFocusContinuous)),
So, I believe there is something in the data in the file itself which overrules the default definition.

I didn't write Exiv2, and there are parts of the code which I have never explored. The decoding of MakerNotes is one of those black holes.

There is a less brutal change that could be made to the code in tags_int.hpp:

const TagDetails* td = find(array, value.toLong());
This code reads the tag definition from canonmn.cpp and therefore can discover that although value.typeId() == 3 == unsignedShort , its default is signedShort and should be converted.

My background is writing compilers and interpreters. I strongly dislike this kind of hack. If the data type is recorded by Canon as unsignedShort, it is not the job of Exiv2 to change it on the fly. I will not submit such a change (nor the hack in this report). I've tried setting the type explicitly and this is not successful.

824 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -M"set Exif.CanonCs.FocusContinuous SShort -1" cs.jpg
825 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pa -K Exif.CanonCs.FocusContinuous cs.jpg
Exif.CanonCs.FocusContinuous                 Short       1  (65535)
826 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -M"set Exif.CanonCs.FocusContinuous Short -1" cs.jpg
Warning: Exif.CanonCs.FocusContinuous: Failed to read Short value "-1"

The road to resolving this is for me to master the Canon MakerNote decoder and I don't have time to undertake that at the moment. If Andreas wrote that code and can give further insight, that would be very helpful.

I do have another constructive change to make here concerning 8 = Manual. I have submitted that change: r4365

822 rmills@rmillsmbp:~/gnu/exiv2/trunk $ cp /Users/rmills/gnu/exiv2/trunk/test/tmp/20030925_201850.jpg cs.jpg
823 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pa -K Exif.CanonCs.FocusContinuous cs.jpg
Exif.CanonCs.FocusContinuous                 Short       1  (65535)
827 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -M"set Exif.CanonCs.FocusContinuous Short 8" cs.jpg
828 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pa -K Exif.CanonCs.FocusContinuous cs.jpg
Exif.CanonCs.FocusContinuous                 Short       1  Manual
829 rmills@rmillsmbp:~/gnu/exiv2/trunk $ exiv2 -pv -K Exif.CanonCs.FocusContinuous cs.jpg
0x0020 CanonCs      FocusContinuous             Short       1  8

I'm not sure I've understood your comment about interpreting -1 as Single. If we don't know the meaning of -1, we shouldn't invent a meaning.

As I am trying to get closed on v0.26, I'm don't have time to investigate the SShort/Short contradiction at this time. I'm therefore marking this as "Resolved".

#2 Updated by Sridhar Boovaraghavan 11 months ago

Hi Robin,

Thanks for looking into this at short notice.

I agree that the "hack" that returns -1 is not the right fix to do.

I also agree with the addition of 8 (Manual).

However, would you mind keeping the bug open or another bug that tries to resolve the -1 issue?

You don't need to target that bug for 0.26, but a later version and assign it to someone else that could look at it/possibly Andreas? (I didn't have any luck in contacting him about another matter a while back regarding VS2015 projects).

Thanks,
Sridhar

#3 Updated by Robin Mills 11 months ago

  • Subject changed from Exif.CanonCs.FocusContinuous comes out as unsigned short to Exif.CanonCs.FocusContinuous = 8 = Manual
  • Status changed from Resolved to Closed

Thanks for opening issue #1203 to deal with the SShort/Short contradiction. I'll deal with this in v0.27.

Can you confirm that you are happy to value -1/63535 is not translated as "Single".

#4 Updated by Sridhar Boovaraghavan 11 months ago

Yes, I am fine with -1 not being translated as Single for now.

Let us address it as part of http://dev.exiv2.org/issues/1203 where I have proposed a slightly different solution for it.

Thanks,
Sridhar

#5 Updated by Robin Mills 11 months ago

r4370 Update test suite.

#6 Updated by Sridhar Boovaraghavan 11 months ago

Hi Robin,

When you changed this line TagInfo(0xffff, "(UnknownCanonCsTag)", "(UnknownCanonCsTag)", N_("Unknown Canon Camera Settings 1 tag"), canonCsId, makerTags, signedShort, 1, printValue)

from unsignedShort to signedShort, wouldn't -1 be a better value than 0xffff?

Also, there are a few other values in that list that are unsignedShort - why not change them too?

Thanks,
Sridhar

#7 Updated by Robin Mills 11 months ago

Well spotted, Sridhar. I should not have changed canonmn.cpp line 1238 Total blunder. I've reverted the change to line 1238 and retained the 'Manual/8' fix. r4375

I only want to deal with the 8/Manual matter in this issue. The 'generic fix' for the SShort/Short contradiction is the subject of #1203. I don't have time at the moment to spend on #1203 because my priority is to work with Ben on #1199 and Gilles on #1207 and to get v0.26 finished.

#8 Updated by Sridhar Boovaraghavan 11 months ago

Yes, that's better. Thanks.

Also available in: Atom PDF

Redmine Appliance - Powered by TurnKey Linux