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
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | exif.473140.patch | 818 bytes | raintonr |
| #11 | exif-geotag_and_E_ALL_fixes.20090929.patch | 6.49 KB | dman |
| #10 | exif-geotag_and_E_ALL_fixes.20090929.patch | 6.85 KB | dman |
| #5 | exif.module.560710.473140.patch | 3.67 KB | raintonr |
| #1 | exif-read-exif-data.patch | 16.3 KB | mcjim |
Comments
Comment #1
mcjim commentedThis 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:
and the new code returns an array like this:
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.
Comment #2
radj commentedThis 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.
Comment #3
raintonr commentedI'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_nodeapiand confirmed that the EXIF data was being pulled from image file and being placed in the node object, however, when looking in the database (incontent_type_imagetable) 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
presaveop instead ofinsertorupdateas described here:http://api.drupal.org/api/function/hook_nodeapi/6
Comment #4
raintonr commentedFollowing 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_storageis called when the$opisupdate. What I said above in #3 about usingpresavestands then (and firingnodeapion that instead ofupdateorinsertworks). Either do this, or have the EXIF module at a lower weight than CCK.Comment #5
raintonr commentedThe 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/generalpage and then processing files. Hopefully it is less confusing. IMHO we should add the 'keyword' to each 'key' to make things even more obvious.Comment #6
rapsli commentedhi 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?
Comment #7
dman commentedI 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.
Comment #8
rapsli commentedoky. I'll take care of the code style. I'll post it here, as soons as I got the dev version up to date.
Comment #9
rapsli commentedadded clean code to dev.
@dman you can take over and add the patch
Comment #10
dman commentedCool, 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
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
Comment #11
dman commentedSorry, left a debug and some whitespace in that one!
This one instead:
Comment #12
rapsli commentedapplied patch to head... can some people confirm the fix?
Comment #13
raintonr commentedToday 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_reformatnotice thatgps_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.
Comment #14
rapsli commentedcommitted to head.