[Bug 764093] New: Wrongful parse of ISO tag in gstexif

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=764093

            Bug ID: 764093
           Summary: Wrongful parse of ISO tag in gstexif
    Classification: Platform
           Product: GStreamer
           Version: 1.x
                OS: All
            Status: NEW
          Severity: minor
          Priority: Normal
         Component: gst-plugins-base
          Assignee: gstreamer-bugs at lists.freedesktop.org
          Reporter: ptsneves at gmail.com
        QA Contact: gstreamer-bugs at lists.freedesktop.org
     GNOME version: ---

Created attachment 324607
  --> https://bugzilla.gnome.org/attachment.cgi?id=324607&action=edit
A proposed patch.

The comments below take into account the standard description summary found in 
http://www.sno.phy.queensu.ca/~phil/exiftool/TagNames/EXIF.html

According to the site above:
0x8827 is ISO or PhotographicSensitivity in EXIF2.3
0x8833 is ISOSpeed (dependent of SensitivityType in ExifISOTag)

According to gstexif.c:
0x8827 is EXIF_TAG_PHOTOGRAPHIC_SENSITIVITY
0x8833 is EXIF_TAG_ISO_SPEED

Then we have following in gstexif.c at 373
/* don't need the serializer as we always write the iso speed alone */
   {GST_TAG_CAPTURING_ISO_SPEED, EXIF_TAG_PHOTOGRAPHIC_SENSITIVITY,
        EXIF_TYPE_SHORT, 0, NULL,
      deserialize_add_to_pending_tags},

   {GST_TAG_CAPTURING_ISO_SPEED, EXIF_TAG_SENSITIVITY_TYPE, EXIF_TYPE_SHORT, 0,
       serialize_sensitivity_type, deserialize_sensitivity_type},
   {GST_TAG_CAPTURING_ISO_SPEED, EXIF_TAG_ISO_SPEED, EXIF_TYPE_LONG, 0, NULL,
      NULL},

Which means that EXIF_TAG_PHOTOGRAPHIC_SENSITIVITY will have deferred parsing
depending on the EXIF_TAG_SENSITIVITY_TYPE which is wrong as seen from the
relation established before in the site.

Instead, the Tag dependent on EXIF_TAG_SENSITIVITY_TYPE should be
EXIF_TAG_ISO_SPEED which is not happening.

This creates a bug that results in the ISO parameter never being parsed. A
scenario where this happens is if Sensitivity Type is different than 3 (ISO
Speed) and instead is 2 (Recommended Exposure Index). What will happen is that
no EXIF_TAG_ISO_SPEED (0X8833) will exist because the type is 2 and the
deferred EXIF_TAG_PHOTOGRAPHIC_SENSITIVITY (0x8827) was discarded in the
SensitivityType parsing because type is different than 3. No more ISO
information can be retrieved from the file.

So assuming we now correctly make the EXIF_TAG_ISO_SPEED dependent on the
EXIF_TAG_SENSITIVITY_TYPE and leave the EXIF_TAG_PHOTOGRAPHIC_SENSITIVITY
correctly represent the GST_TAG_CAPTURING_ISO_SPEED the problem outlined before
does not happen because a standard compliant exif field will not even have
EXIF_TAG_ISO_SPEED if the EXIF_TAG_SENSITIVITY_TYPE is different than 3. For
the example above where the EXIF_TAG_SENSITIVITY_TYPE is 2, the exif will have
a RecommendedExposureIndex (0x8832) tag which is currently not handled. The
EXIF_TAG_PHOTOGRAPHIC_SENSITIVITY would then be parseable and not dependent on
anything.

The problem is that EXIF_TAG_PHOTOGRAPHIC_SENSITIVITY is of size type
EXIF_TYPE_SHORT which for a reason I do not understand does not have a function
that parses shorts like parse_exif_<type>_tag. This happens with other types. I
propose thus the following:

static void parse_exif_short_tag(GstExifReader * reader, const GstExifTagMatch 
tag, guint32 count, guint32 offset, const guint8 * offset_as_data)

This way in gstexif.c at parse_exif_long_tag we can parse EXIF_TYPE_SHORTs.

With everything together the problem is solved on my side.

I attach a proposed patch.

Hoping I was clear and thankful for Gstreamer
Paulo Neves

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.