[KPhotoAlbum] More patches

[Robert Krawitz]

On Sun, 9 Aug 2015 12:09:01 -0400, Robert Krawitz wrote:
> On Sun, 9 Aug 2015 11:03:34 -0400, Robert Krawitz wrote:
>> On Sat, 08 Aug 2015 13:17:46 +0200, Tobias Leupold wrote:
>>> Hi Robert :-)
>>>
>>> I just wanted to try out your patches, but Miika is right: at least for the link-images patch, your code is _removed_ by it and not added, so I can't apply it.
>>>
>>> Would you be so kind to recreate the patches in the right "direction", so that we don't have to apply the changes by hand? Would be nice :-)
>>
>> I don't really like the current lens EXIF logic.  On many Canon
>> cameras, Lens will work better than LensType.  For example, on the
>> 300D:
>>
>> [1(rlk)|{!10}<dsl092-065-009>/altmnt/images/300d/dcim/101canon]
>> $ exiv2 -pa img_0108.jpg |grep Lens
>> Exif.CanonCs.LensType                        Short       1  (65535)
>> Exif.CanonCs.Lens                            Short       3  28.0 - 135.0 mm
>
> The other logic problem I see with this is that it iterates just once
> over the EXIF data (which is not in any defined order), so which
> clause triggers (LensModel, LensIDNumber, LensType) depends upon the
> order in which the iterator happens to return on any given instance.

The logic is still not right.  This structuring, in particular, is
incorrect:

    for (Exiv2::ExifData::const_iterator it = data.begin(); it != data.end(); ++it)
    {
        const QString datum = QString::fromLatin1(it->key().c_str());

        if (datum.endsWith(QString::fromLatin1(".LensModel")))
        {
            canonHack = false;
            value = QString::fromUtf8(it->toString().c_str());
            // we can break here since Exif.Photo.LensModel should be bound first
            break;
        }

        if (datum.endsWith(QString::fromLatin1(".LensIDNumber")))
        {
            canonHack = false;
            value = QString::fromUtf8(it->print(&data).c_str());
            continue;
        }

        if (datum.endsWith(QString::fromLatin1(".LensType")))
        {
            if (value.isEmpty())
            {
                canonHack = (datum == QString::fromLatin1("Exif.CanonCs.LensType"));
                value = QString::fromUtf8(it->print(&data).c_str());
            }
        }
    }

The problem is that the EXIF tags are not returned in any particular
order, so you'll pick up whichever tag happens to be returned first.
It needs to be structured as below.

Also, the canonHack thing isn't the right way to go about it; other
cameras may return bogus values.  Those need to be detected and
ignored.

Finally, Exif.CanonCS.Lens is a triple of elements.  If it's converted
directly to a string, it will look something like

135 18 1

It needs to use the logic in my original patch.

    for (Exiv2::ExifData::const_iterator it = data.begin(); it != data.end(); ++it)
    {
        const QString datum = QString::fromLatin1(it->key().c_str());

        if (datum.endsWith(QString::fromLatin1(".LensModel")))
        {
            canonHack = false;
            value = QString::fromUtf8(it->toString().c_str());
            // we can break here since Exif.Photo.LensModel should be bound first
            break;
        }
    }
    
    for (Exiv2::ExifData::const_iterator it = data.begin(); it != data.end(); ++it)
    {
        if (datum.endsWith(QString::fromLatin1(".LensIDNumber")))
        {
            canonHack = false;
            value = QString::fromUtf8(it->print(&data).c_str());
            continue;
        }
    }

    for (Exiv2::ExifData::const_iterator it = data.begin(); it != data.end(); ++it)
    {
        if (datum.endsWith(QString::fromLatin1(".LensType")))
        {
            if (value.isEmpty())
            {
                canonHack = (datum == QString::fromLatin1("Exif.CanonCs.LensType"));
                value = QString::fromUtf8(it->print(&data).c_str());
            }
        }
    }

-- 
Robert Krawitz                                     <rlk at alum.mit.edu>

***  MIT Engineers   A Proud Tradition   http://mitathletics.com  ***
Member of the League for Programming Freedom  --  http://ProgFree.org
Project lead for Gutenprint   --    http://gimp-print.sourceforge.net

"Linux doesn't dictate how I work, I dictate how Linux works."
--Eric Crampton