Hi,

I just migrated a site from Drupal 5.18 to Drupal 6.12 and switched the exif module accordingly. After reading the README.TXT I created the sample CCK fields "field_exif_exposuretime" (as text) and "field_ifd0_datetime" (as date), but nothing is displayed, neither in the node page itself nor in the node editing form.

On /admin/settings/exif/config, the image content type is enabled ("image" from image module); on /admin/settings/exif/visibility I played with the settings "Show CCK Fields on Node-Creation form" and "Show CCK Fields on Node-Update form", but whatever is enabled, no exif data is displayed anywhere.

To my understanding, the D6 version doesn't need PEL anymore, at least it can't be configured anymore; maybe there are other hidden requirements I might have missed? Or does the module require several cron runs to extract the data (this site contains >25k image nodes)? Or is HEAD simply inoperable at the moment?

Thanks & greetings,
-asb

Comments

mcjim’s picture

Priority: Normal » Critical
Status: Active » Needs review
StatusFileSize
new16.3 KB

This appears to be a bug in the 6 dev code.
I've attached a patch which fixes it for now.

More detail

The method which extracts the EXIF data (readExifTags()) is using the function exif_read_data(), but without setting the third argument to TRUE. This is necessary to return the EXIF data in a consistent manner.
The original code was returning an array like this:

Array
(
    [FileName] => Image007.jpg
    [FileDateTime] => 1247050286
    [FileSize] => 35631
    [FileType] => 2
    [MimeType] => image/jpeg
    [SectionsFound] => 
    [COMPUTED] => Array
        (
            [html] => width="640" height="480"
            [Height] => 480
            [Width] => 640
            [IsColor] => 1
        )

)

and the new code returns an array like this:

Array
(
    [FILE] => Array
        (
            [FileName] => Image011.jpg
            [FileDateTime] => 1247050646
            [FileSize] => 32899
            [FileType] => 2
            [MimeType] => image/jpeg
            [SectionsFound] => 
        )

    [COMPUTED] => Array
        (
            [html] => width="640" height="480"
            [Height] => 480
            [Width] => 640
            [IsColor] => 1
        )

)

The original code was only pulling in the non-array values (i.e. FileName, FileDateTime, etc) and ignoring the nested arrays (COMPUTED, EXIT, etc).
The attached patch includes this change and updates the rest of the code to understand the different format.

I also ran the module through coder and formatted accordingly, so the patch is larger than the bug-fix would imply.

radj’s picture

This patch worked for me. However, I'm not so sure if it's all safe since I'm not familiar with coding guidelines so I can't change the status.

I have a suggestion: why don't you let the sample data in admin/settings/exif/general pull out using the way it pulls out during node update to double check if the CCK fields will pull them out right? When I encountered this error, I replaced the sample.jpg in the exif module folder to test the "admin/settings/exif/general" page's correctness and all the data were shown, including the COMPUTED fields from the replaced sample.jpg file. From there, I knew there was an inconsistency between the cck field exif code and the admin page code.

raintonr’s picture

I'm using EXIF 6.x-1.x-dev (2009-Apr-13) with image 6.x-1.x-dev (2009-Jun-23) and CCK 6.x-2.4

Created 3 CCK fields (all text) 'field_ifd0_model', 'field_gps_gpslatitude' & 'field_gps_gpslongitude'.

After submitting an image with this information within nothing was appearing on the node display though.

I placed some debug messages inside exif_nodeapi and confirmed that the EXIF data was being pulled from image file and being placed in the node object, however, when looking in the database (in content_type_image table) note the fields are null.

On the EXIF admin settings turned the visibility of these fields on and was able to manually enter data which is saved.

This is very odd. It seems that CCK is saving manually entered data when the fields are visible, but not when it's populated by EXIF module.

I'm not convinced that this module should be using the presave op instead of insert or update as described here:

http://api.drupal.org/api/function/hook_nodeapi/6

"presave": The node passed validation and is about to be saved. Modules may use this to make changes to the node before it is saved to the database.

raintonr’s picture

Following up on this I've discovered the following:

- Trying to use GPS co-ordinates from the EXIF causes the following error and no data (not just the erroneous part) to be saved: mysql_real_escape_string() expects parameter 1 to be string, array given in ...drupal-6.12/includes/database.mysql.inc on line 321.. This is because the decode of EXIF GPS co-ords returns a 3 element array and not a string. I will raise a separate issue for this.

- Putting lots of debug info in both CCK and a dummy module just to see when various things were happening I discovered that content_storage is called when the $op is update. What I said above in #3 about using presave stands then (and firing nodeapi on that instead of update or insert works). Either do this, or have the EXIF module at a lower weight than CCK.

raintonr’s picture

StatusFileSize
new3.67 KB

The enclosed patch works for me.

Update Actually - I see a problem with the lat/lon refs. Firstly if lat/lon are collected we should force the ref to be, and secondly, the check looking at ref. is broken. Sorry, will fix this ASAP.

Note, the only change that applies to this issue is the update/insert -> presave change. However, Geo tags would still cause a problem (as discussed in #4) and that's what the majority of this patch caters for.

The Geo problem is discussed here: #560710: exif parses geo data into a sub-array - causes errors. and the enclosed patch should correct that.

This patch also uses a common mechanism for showing names on the admin/settings/exif/general page and then processing files. Hopefully it is less confusing. IMHO we should add the 'keyword' to each 'key' to make things even more obvious.

rapsli’s picture

Status: Needs review » Needs work

hi there
I'm trying to get the module up to date again. Unfortunately there patch doesn't apply anymore. I tried to recreate it, but didn't suceed... can someone make a new patch, that applies to the latest dev version?

dman’s picture

I think the patch is an OK approach. I could try adding it by hand - if the current CVS-dev is up-to-date with code style fixes? I couldn't roll good patches when it had tabs and things in it. Let me know when it gets a clean report from coder.module and I'll join in.

rapsli’s picture

oky. I'll take care of the code style. I'll post it here, as soons as I got the dev version up to date.

rapsli’s picture

added clean code to dev.
@dman you can take over and add the patch

dman’s picture

Status: Needs work » Needs review
StatusFileSize
new6.85 KB

Cool, thanks for the code style cleanup! Here this is, includes the above patch for geodata (inserted by hand with a few tweaks) and some fixes for PHP E_ALL strict code compliance.

coder.module checked.
Works as expected now.
Noted two TODOs : throws an error if an image.nose contains a PDF, Needs some layout love to do something with those binary blobs.
I have suspicions that different versions of exif may produce different results, but I'm not able to test that. My test was on

PHP Version 5.2.8
EXIF Version 	1.4 $Id: exif.c,v 1.173.2.5.2.26 2008/08/03 12:11:13 jani Exp $
Supported EXIF Version 	0220 

CHANGELOG:
> 2009/09/24 dman
> * Cleaned up PHP E_ALL notices (undefined variables) in a few places, including exif.class
> * Typo in exif_nodeapi()
> * Applied fix for array-structured Geo tags http://drupal.org/node/473140

dman’s picture

StatusFileSize
new6.49 KB

Sorry, left a debug and some whitespace in that one!
This one instead:

rapsli’s picture

applied patch to head... can some people confirm the fix?

raintonr’s picture

StatusFileSize
new818 bytes

Today I did this:

- Downloaded the latest dev version of EXIF (dated 29 Sep).
- Tried to run on an installation with the following CCK fields on the image content type: field_gps_gpslatitude, field_gps_gpslatituderef, field_gps_gpslongitude, field_gps_gpslongituderef
- Created an image node using a Geo-coded image.

This did not work.

Taking a look at the code and the contents of arguments passed to _exif_reformat notice that gps_ needed pre-pending to the keys. The enclosed patch makes that fix and once done all is well.

Incidentally that same issue was spotted here: http://drupal.org/node/178370#comment-2167176

But basically, the attached patch should fix this up so please have a look and confirm/commit ASAP.

rapsli’s picture

Status: Needs review » Fixed

committed to head.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.