Incorrectly extracting Canon AFInfo?

Started by Hayo Baan, July 01, 2019, 11:36:30 AM

Previous topic - Next topic

Hayo Baan

Hi Phil,

I writing some code to decode Canon AF Info (amongst others) and have used the exiftool code as reference (of course ;)). I think you've made a mistake in the code though.

In %Image::ExifTool::Canon::AFInfo:
11 => [
        {
            Name => 'PrimaryAFPoint',
            Condition => q{
                $$self{Model} !~ /EOS/ and
                (not $$self{AFInfoCount} or $$self{AFInfoCount} != 36)
            },
        },
        {
            # (some PowerShot 9-point systems put PrimaryAFPoint after 8 unknown values)
            Name => 'Canon_AFInfo_0x000b',
            Condition => '$$self{Model} !~ /EOS/',
            Format => 'int16u[8]',
            Unknown => 1,
        },
        # (serial processing stops here for EOS cameras)
    ],
    12 => 'PrimaryAFPoint',


Apart from the fact that the condition part (not $$self{AFInfoCount} or $$self{AFInfoCount} != 36) would be clearer if rewritten as $val{0} != 9 since this conveys the reasoning better, the above code does not decode PrimaryAFPoint for e.g. a Canon 5D while the data is present (you now only interpret 39 of the 40 available int16u elements). I don't fully understand why this is the case since you do have 12 => 'PrimaryAFPoint', but I guess this doesn't fire since the 11 block didn't result in any output. If that's the reason for the ommision, the first condition could be rewritten as $$self{Model} =~ /EOS/ or $val{0} != 9 to solve this, I think. The condition for the second part for element 11 could then go as well but you'll need to make 12 conditional again :)

In %Image::ExifTool::Canon::AFInfo2:
    13 => [
        {
            Name => 'AFPointsSelected',
            Condition => '$$self{Model} =~ /EOS/',
            Format => 'int16s[int(($val{2}+15)/16)]',
            PrintConv => 'Image::ExifTool::DecodeBits($val, undef, 16)',
        },
        {
            Name => 'Canon_AFInfo2_0x000d',
            Format => 'int16s[int(($val{2}+15)/16)+1]',
            Unknown => 1,
        },
    ],
    14 => {
        # usually, but not always, the lowest number AF point in focus
        Name => 'PrimaryAFPoint',
        Condition => '$$self{Model} !~ /EOS/ and not $$self{AFInfo3}', # (not valid for G1XmkII)
    },


For non-EOS models, this skips an additional int16 element as part of 13. But isn't that where the PrimaryAFPoint (14) is stored?
Also in 14 you don't extract PrimaryAFPoint in case of EOS, is that correct? Also the test not $$self{AFInfo3} to (presumably?) test for G1XmkII isn't very clear. Shouldn't that be a match of model with "G1XmkII" (or whatever the model name is in that case)?
Hayo Baan – Photography
Web: www.hayobaan.nl

Hayo Baan

I noticed another omission, but here I don't know the what the solution could be. I found a file from a Canon 5Dii and there the AFInfo2 block is in fact 48 int16 elements, but you only decode 46. Maybe we can assume element 46 is again the PrimaryAFPoint, but then we still have element 47 to account for...
Hayo Baan – Photography
Web: www.hayobaan.nl

Hayo Baan

And another: the Canon EOS 7D has 91 int16 data elements (19 AF points), but only 89 are extracted.
I don't know what the additional elements are for.

Looking at the data again, I actually don't think EOS cameras have a PrimaryAFPoint (the numbers don't make sense for the cameras I looked at), so your code is fully correct after all. However, perhaps you still could extract the additional elements (into an "unknown" tag).
Hayo Baan – Photography
Web: www.hayobaan.nl

Phil Harvey

Hi Hayo,

Thanks for the suggestions.

Quote from: Hayo Baan on July 01, 2019, 11:36:30 AM
Apart from the fact that the condition part (not $$self{AFInfoCount} or $$self{AFInfoCount} != 36) would be clearer if rewritten as $val{0} != 9 since this conveys the reasoning better,

I tried changing it to this and it broke the decoding for many PowerShot models.  Maybe I did something wrong.  Did you test this with the Canon sample images?

Quotethe above code does not decode PrimaryAFPoint for e.g. a Canon 5D while the data is present

Correct.  EOS cameras currently do not decode this.

Quote(you now only interpret 39 of the 40 available int16u elements). I don't fully understand why this is the case since you do have 12 => 'PrimaryAFPoint', but I guess this doesn't fire since the 11 block didn't result in any output.

Correct.  This is a special SerialData block, which processes the data serially and aborts on the first invalid tag.

QuoteIf that's the reason for the ommision, the first condition could be rewritten as $$self{Model} =~ /EOS/ or $val{0} != 9

It looks to me as if the code purposely doesn't decode this for EOS cameras.  I don't know why now, but I'm sure there was a good reason.

Quoteto solve this, I think. The condition for the second part for element 11 could then go as well but you'll need to make 12 conditional again :)

I'm a bit confused by what you want.  Could you post your tag definitions from tag 11 onward?

QuoteIn %Image::ExifTool::Canon::AFInfo2:
[...]

For non-EOS models, this skips an additional int16 element as part of 13. But isn't that where the PrimaryAFPoint (14) is stored?

This is a special serial data structure.  14 is not the offset, it is just a sequence number.  This code for tag 13 skips an extra unkown int16s value because apparently PrimaryAFPoint comes after that.

Quote
Also in 14 you don't extract PrimaryAFPoint in case of EOS, is that correct?

If there is still data after tag 13 is extracted, then tag 14 will be extracted even for EOS models.

QuoteAlso the test not $$self{AFInfo3} to (presumably?) test for G1XmkII isn't very clear. Shouldn't that be a match of model with "G1XmkII" (or whatever the model name is in that case)?

It was done this way in an attempt to be more compatible with future models since I don't have the resources to test the AF points with all models.  G1XmkII is just an example of one camera where this is true, but I anticipate there are others and an guessing AFInfo3 may be an indicator of this.

- Phil
...where DIR is the name of a directory/folder containing the images.  On Mac/Linux/PowerShell, use single quotes (') instead of double quotes (") around arguments containing a dollar sign ($).

Hayo Baan

Hi Phil,

Awesome, thanks for the additional info! Really helpful :)
I think I got the Canon stuff sorted now. Now on to Nikon (which is MUCH harder, I think).
(FYI, I'm writing C++ code to extract AF focus point info from image files. Ultimately this  should lead to being able to show the used focus points in software; a much sought after feature, presumably).

Quote from: Phil Harvey on July 02, 2019, 08:20:57 AM
Hi Hayo,

Thanks for the suggestions.

Quote from: Hayo Baan on July 01, 2019, 11:36:30 AM
Apart from the fact that the condition part (not $$self{AFInfoCount} or $$self{AFInfoCount} != 36) would be clearer if rewritten as $val{0} != 9 since this conveys the reasoning better,

I tried changing it to this and it broke the decoding for many PowerShot models.  Maybe I did something wrong.  Did you test this with the Canon sample images?
Indeed, I tested this and as it turns out you only need to skip the 8 elements if there are 9 AF points and the size is 36 (with 9 AF points you would normally just need 28 elements). So it looks like your code is fine (though perhaps an additional check for the number of AF points in this case is suitable, though I think this works fine as is).

Quote from: Phil Harvey on July 02, 2019, 08:20:57 AM
Quotethe above code does not decode PrimaryAFPoint for e.g. a Canon 5D while the data is present

Correct.  EOS cameras currently do not decode this.

Quote(you now only interpret 39 of the 40 available int16u elements). I don't fully understand why this is the case since you do have 12 => 'PrimaryAFPoint', but I guess this doesn't fire since the 11 block didn't result in any output.

Correct.  This is a special SerialData block, which processes the data serially and aborts on the first invalid tag.

QuoteIf that's the reason for the ommision, the first condition could be rewritten as $$self{Model} =~ /EOS/ or $val{0} != 9

It looks to me as if the code purposely doesn't decode this for EOS cameras.  I don't know why now, but I'm sure there was a good reason.
Yes, there actually was since I later found that that values doesn't seem to be an PrimaryAFPoint anyway (its 0xffff in the cases I found).

Quote from: Phil Harvey on July 02, 2019, 08:20:57 AM
Quoteto solve this, I think. The condition for the second part for element 11 could then go as well but you'll need to make 12 conditional again :)

I'm a bit confused by what you want.  Could you post your tag definitions from tag 11 onward?
Forget my suggestions; your code does the correct thing; PrimaryAFPoint should not be decoded.

Quote from: Phil Harvey on July 02, 2019, 08:20:57 AM
QuoteIn %Image::ExifTool::Canon::AFInfo2:
[...]

For non-EOS models, this skips an additional int16 element as part of 13. But isn't that where the PrimaryAFPoint (14) is stored?

This is a special serial data structure.  14 is not the offset, it is just a sequence number.  This code for tag 13 skips an extra unkown int16s value because apparently PrimaryAFPoint comes after that.
Quote
Also in 14 you don't extract PrimaryAFPoint in case of EOS, is that correct?

If there is still data after tag 13 is extracted, then tag 14 will be extracted even for EOS models.
Actually not since its condition would be false in that case ;)

Quote from: Phil Harvey on July 02, 2019, 08:20:57 AM
QuoteAlso the test not $$self{AFInfo3} to (presumably?) test for G1XmkII isn't very clear. Shouldn't that be a match of model with "G1XmkII" (or whatever the model name is in that case)?

It was done this way in an attempt to be more compatible with future models since I don't have the resources to test the AF points with all models.  G1XmkII is just an example of one camera where this is true, but I anticipate there are others and an guessing AFInfo3 may be an indicator of this.
Good thinking, have coded it the same myself now.

So all in all, it turns out your code was fully correct, it was just that some things looked or worked differently from what I expected from the code/comments. Thanks for your patience.
Hayo Baan – Photography
Web: www.hayobaan.nl