PHP native EXIF parser has a lot of issues, and sometimes is not even available.
We should replace `exif_read_data` by a 3rd party and autonomous library (do not rely on external binaries like exiftool).
I found https://github.com/lsolesen/pel (using namespaces, perhaps it's time for Piwigo to switch to PHP 5.3...).
Perhaps there are more.
Need to do a benchmark before making a choice.
(related to a very old ticket #338)
<!-- github-sync-id:113215240 -->
(using namespaces, perhaps it's time for Piwigo to switch to PHP 5.3...)
That's definitely possible. I'm perfectly OK with that. 5.4 could easily become a requirement now.
I don't know if PEL is the best way to go, but using a better exif parser could soon become an "important task". Several recent issue on GPS parsing, when makernotes are in the EXIF metadata.
> (using namespaces, perhaps it's time for Piwigo to switch to PHP 5.3...)
That's definitely possible. I'm perfectly OK with that. 5.4 could easily become a requirement now.
I don't know if PEL is the best way to go, but using a better exif parser could soon become an "important task". Several recent issue on GPS parsing, when makernotes are in the EXIF metadata.
<!-- github-comment-id:258224916 -->
Hi, I do not want to interfere, but it seems to me that all Exif libraries for PHP are no longer maintained (last commit 2013 or even 2011).
On the other hand, the "de-facto-standard" in Exif parsing (Exiftool) is really a Perl-library http://www.sno.phy.queensu.ca/~phil/exiftool/
So, if you do not want to call an external binary and probably do not want to maintain a Exif parsing library in PHP yourself:
Wouldn't it be possible to use the exiftool-library via PECL ? https://pecl.php.net/package/perl
On the other hand it would be possible (and in fact I have already done so on my own server) to use exiftool if present and fall back to the php implementation if not - these changes are just a few lines of code in 2 or 3 files ...
Hi, I do not want to interfere, but it seems to me that all Exif libraries for PHP are no longer maintained (last commit 2013 or even 2011).
On the other hand, the "de-facto-standard" in Exif parsing (Exiftool) is really a Perl-library http://www.sno.phy.queensu.ca/~phil/exiftool/
So, if you do not want to call an external binary and probably do not want to maintain a Exif parsing library in PHP yourself:
Wouldn't it be possible to use the exiftool-library via PECL ?
https://pecl.php.net/package/perl
On the other hand it would be possible (and in fact I have already done so on my own server) to use exiftool if present and fall back to the php implementation if not - these changes are just a few lines of code in 2 or 3 files ...
<!-- github-comment-id:259758876 -->
The main target of Piwigo is still a majority of shared hosts with limited capabilities in term of extensions and binaries.
But the lib in my first message was updated 7 months ago, not in 2013...
The main target of Piwigo is still a majority of shared hosts with limited capabilities in term of extensions and binaries.
But the lib in my first message was updated 7 months ago, not in 2013...
<!-- github-comment-id:259928224 -->
I stand corrected - it seems that isolesen re-started developing and fixing bugs in his library (or never stopped and just didn't update the old homepage).
And I perfectly understand the primary target of piwigo - this is why I tried to think of ways to include exiftool (which I know works very well for all kinds of images (even videos and with severely "broken" metadata) ) without breaking the possibility of shared hosting. As I am not "fluent" in PHP I took a look at the details of PECL and PECL::perl yesterday and have to agree - this is probably not for shared hosting (at least not when there is not even a PHP::exif available ...)
Of course a stable and good working library in "native PHP" would be the best solution. If PEL is / will be that library - great! I definitely look forward to having reliable Exif-Support without my "personal patches" (i.e. using exiftool in the background).
I stand corrected - it seems that isolesen re-started developing and fixing bugs in his library (or never stopped and just didn't update the old homepage).
And I perfectly understand the primary target of piwigo - this is why I tried to think of ways to include exiftool (which I know works very well for all kinds of images (even videos and with severely "broken" metadata) ) without breaking the possibility of shared hosting. As I am not "fluent" in PHP I took a look at the details of PECL and PECL::perl yesterday and have to agree - this is probably not for shared hosting (at least not when there is not even a PHP::exif available ...)
Of course a stable and good working library in "native PHP" would be the best solution. If PEL is / will be that library - great! I definitely look forward to having reliable Exif-Support without my "personal patches" (i.e. using exiftool in the background).
<!-- github-comment-id:259964468 -->
Hmm, not as far as I can tell. Perhaps I will cleanup and submit my "exiftool-Patches".
They use exiftool if present and fall back to the default behaviour when not ... so, only improvements there ;-)
One thing about PEL that I don't like is that it cannot parse metadata for movies - and as far as I can see stores the whole image in memory for parsing - that could be a problem for smaller virtual hosting packages ...
Of course, "using exiftool if present, fall back to PEL if not" would be best - and circumvent the broken inbuilt PHP exif support ...
Hmm, not as far as I can tell. Perhaps I will cleanup and submit my "exiftool-Patches".
They use exiftool if present and fall back to the default behaviour when not ... so, only improvements there ;-)
One thing about PEL that I don't like is that it cannot parse metadata for movies - and as far as I can see stores the whole image in memory for parsing - that could be a problem for smaller virtual hosting packages ...
Of course, "using exiftool if present, fall back to PEL if not" would be best - and circumvent the broken inbuilt PHP exif support ...
<!-- github-comment-id:314406377 -->
@IT-Sumpfling Please do so! I did hack a bit on my end as well so we should be able to come up with a decent support for exiftool (I created a small plugin to maintain exiftool path and parameters...).
@IT-Sumpfling Please do so! I did hack a bit on my end as well so we should be able to come up with a decent support for exiftool (I created a small plugin to maintain exiftool path and parameters...).
<!-- github-comment-id:314450592 -->
check out http://piwigo.org/ext/extension_view.php?eid=850 and http://piwigo.org/ext/extension_view.php?eid=848 It may help to solve your issues and to build a proper fix if needed
<!-- github-comment-id:314453271 -->
The extensions look interesting - is that the "proper" way to override the builtin exif parsing (can it also the "default" properties like date etc.)?
I will send the exiftool-commandline I use - which "automatically" formats the different values like the builtin ... and uses php output format - so no need to do json-parsing.
The extensions look interesting - is that the "proper" way to override the builtin exif parsing (can it also the "default" properties like date etc.)?
I will send the exiftool-commandline I use - which "automatically" formats the different values like the builtin ... and uses php output format - so no need to do json-parsing.
<!-- github-comment-id:314465379 -->
Who would have guessed it could be so easy??? @IT-Sumpfling If you're interested you might have a look at https://github.com/ThomasDaheim/piwigo-stuff/tree/master/piwigo-exiftool - my attempt to write a complete plugin that allows to customize exiftool path & parameters. Combined with your code to call exiftool that would be all we need.
Who would have guessed it could be so easy??? @IT-Sumpfling If you're interested you might have a look at https://github.com/ThomasDaheim/piwigo-stuff/tree/master/piwigo-exiftool - my attempt to write a complete plugin that allows to customize exiftool path & parameters. Combined with your code to call exiftool that would be all we need.
<!-- github-comment-id:314512512 -->
This would be replaced by your config. I use a boolean variable if exiftool is really available because some other checks depend on this. This check is probably better in the config, too.
- if (!function_exists('read_exif_data'))
+ if (!$useexiftool && !function_exists('read_exif_data'))
{
die('Exif extension not available, admin should disable exif use');
}
The old version stops here if no php exif extensions are available - as we try to be independent of these, that would not be helpful ...
+ if ($useexiftool)
+ {
+ eval('$exif=' . `exiftool -php -c "%+.14f" -s -sep "," -q $filename`);
+ $exif = $exif[0];
+ }
+ else
+ {
+ $exif = @read_exif_data($filename);
+ }
// Read EXIF data
- if ($exif = @read_exif_data($filename) or $exif2 = trigger_change('format_exif_data', $exif=null, $filename, $map))
+ if ($exif or $exif2 = trigger_change('format_exif_data', $exif=null, $filename, $map))
{
if (!empty($exif2))
{
I don't particularly like this part - neither the original nor my changes. but it was the best I could come up with in a hurry which should work in any case and keep close to the original.
What the original did was:
try to read EXIF via PHP
if that fails (empty array) => call format_exif_data with null
if 2. returned values - use them
otherwise, call format_exif_data with the values from read_exif_data (the PHP function)
Honestly I do think that we could get away with calling "format_exif_data" only once - if read_exif_data fails, $exif is empty anyway. But important: here is the "magic line" to call exiftool:
eval('$exif=' . exiftool -php -c "%+.14f" -s -sep "," -q $filename);
will call exiftool in PHP-mode ( -php returns the result as PHP array), formatting GPS-coordinates as floats in the same format as used by piwigo (-c "%+.14f") use "," as list seperator (-sep ",") and use short output format (-s) and be quiet (-q).
Store the output in $exif and evaluate it - bingo - everything is there!
The last part deals with GPS-coordinate parsing:
My patch uses the old code when exiftool is not used (if ! $useexiftool) and just assigns the floats when exiftool is used:
@@ -182,16 +197,27 @@
}
// GPS data
- $gps_exif = array_intersect_key($exif, array_flip(array('GPSLatitudeRef', 'GPSLatitude', 'GPSLongitudeRef', 'GPSLongitude')));
- if (count($gps_exif) == 4)
+ if (! $useexiftool)
+ {
+ $gps_exif = array_intersect_key($exif, array_flip(array('GPSLatitudeRef', 'GPSLatitude', 'GPSLongitudeRef', 'GPSLongitude')));
+ if (count($gps_exif) == 4)
+ {
+ if (
+ is_array($gps_exif['GPSLatitude']) and in_array($gps_exif['GPSLatitudeRef'], array('S', 'N')) and
+ is_array($gps_exif['GPSLongitude']) and in_array($gps_exif['GPSLongitudeRef'], array('W', 'E'))
+ )
+ {
+ $result['latitude'] = parse_exif_gps_data($gps_exif['GPSLatitude'], $gps_exif['GPSLatitudeRef']);
+ $result['longitude'] = parse_exif_gps_data($gps_exif['GPSLongitude'], $gps_exif['GPSLongitudeRef']);
+ }
+ }
+ }
+ else
{
- if (
- is_array($gps_exif['GPSLatitude']) and in_array($gps_exif['GPSLatitudeRef'], array('S', 'N')) and
- is_array($gps_exif['GPSLongitude']) and in_array($gps_exif['GPSLongitudeRef'], array('W', 'E'))
- )
+ if (array_key_exists('GPSLatitude', $exif))
{
- $result['latitude'] = parse_exif_gps_data($gps_exif['GPSLatitude'], $gps_exif['GPSLatitudeRef']);
- $result['longitude'] = parse_exif_gps_data($gps_exif['GPSLongitude'], $gps_exif['GPSLongitudeRef']);
+ $result['latitude'] = floatval($exif['GPSLatitude']);
+ $result['longitude'] = floatval($exif['GPSLongitude']);
}
}
}
So, long enough for one post ... next post will be about "how to do this better" ;-)
@ThomasDaheim
Sorry about the delay, here my "patch" for functions_metadata.inc.php
Github doesn't allow me to upload it, so I'll paste it as text.
Comments between the single blocks ...
```
--- include/functions_metadata.inc.php 2017-04-24 16:16:18.000000000 +0200
+++ include.fixed/functions_metadata.inc.php 2016-06-29 23:51:41.487003981 +0200
@@ -143,14 +143,29 @@
global $conf;
$result = array();
+ $exiftoolpath = '/usr/bin/exiftool';
+ $useexiftool = 0;
+ if(file_exists($exiftoolpath))
+ {
+ $useexiftool = 1;
+ }
```
This would be replaced by your config. I use a boolean variable if exiftool is really available because some other checks depend on this. This check is probably better in the config, too.
```
- if (!function_exists('read_exif_data'))
+ if (!$useexiftool && !function_exists('read_exif_data'))
{
die('Exif extension not available, admin should disable exif use');
}
```
The old version stops here if no php exif extensions are available - as we try to be independent of these, that would not be helpful ...
```
+ if ($useexiftool)
+ {
+ eval('$exif=' . `exiftool -php -c "%+.14f" -s -sep "," -q $filename`);
+ $exif = $exif[0];
+ }
+ else
+ {
+ $exif = @read_exif_data($filename);
+ }
// Read EXIF data
- if ($exif = @read_exif_data($filename) or $exif2 = trigger_change('format_exif_data', $exif=null, $filename, $map))
+ if ($exif or $exif2 = trigger_change('format_exif_data', $exif=null, $filename, $map))
{
if (!empty($exif2))
{
```
I don't particularly like this part - neither the original nor my changes. but it was the best I could come up with in a hurry which should work in any case and keep close to the original.
What the original did was:
1. try to read EXIF via PHP
2. if that fails (empty array) => call format_exif_data with null
3. if 2. returned values - use them
4. otherwise, call format_exif_data with the values from read_exif_data (the PHP function)
Honestly I do think that we could get away with calling "format_exif_data" only once - if read_exif_data fails, $exif is empty anyway. But important: here is the "magic line" to call exiftool:
eval('$exif=' . `exiftool -php -c "%+.14f" -s -sep "," -q $filename`);
will call exiftool in PHP-mode ( -php returns the result as PHP array), formatting GPS-coordinates as floats in the same format as used by piwigo (-c "%+.14f") use "," as list seperator (-sep ",") and use short output format (-s) and be quiet (-q).
Store the output in $exif and evaluate it - bingo - everything is there!
The last part deals with GPS-coordinate parsing:
My patch uses the old code when exiftool is not used (if ! $useexiftool) and just assigns the floats when exiftool is used:
```
@@ -182,16 +197,27 @@
}
// GPS data
- $gps_exif = array_intersect_key($exif, array_flip(array('GPSLatitudeRef', 'GPSLatitude', 'GPSLongitudeRef', 'GPSLongitude')));
- if (count($gps_exif) == 4)
+ if (! $useexiftool)
+ {
+ $gps_exif = array_intersect_key($exif, array_flip(array('GPSLatitudeRef', 'GPSLatitude', 'GPSLongitudeRef', 'GPSLongitude')));
+ if (count($gps_exif) == 4)
+ {
+ if (
+ is_array($gps_exif['GPSLatitude']) and in_array($gps_exif['GPSLatitudeRef'], array('S', 'N')) and
+ is_array($gps_exif['GPSLongitude']) and in_array($gps_exif['GPSLongitudeRef'], array('W', 'E'))
+ )
+ {
+ $result['latitude'] = parse_exif_gps_data($gps_exif['GPSLatitude'], $gps_exif['GPSLatitudeRef']);
+ $result['longitude'] = parse_exif_gps_data($gps_exif['GPSLongitude'], $gps_exif['GPSLongitudeRef']);
+ }
+ }
+ }
+ else
{
- if (
- is_array($gps_exif['GPSLatitude']) and in_array($gps_exif['GPSLatitudeRef'], array('S', 'N')) and
- is_array($gps_exif['GPSLongitude']) and in_array($gps_exif['GPSLongitudeRef'], array('W', 'E'))
- )
+ if (array_key_exists('GPSLatitude', $exif))
{
- $result['latitude'] = parse_exif_gps_data($gps_exif['GPSLatitude'], $gps_exif['GPSLatitudeRef']);
- $result['longitude'] = parse_exif_gps_data($gps_exif['GPSLongitude'], $gps_exif['GPSLongitudeRef']);
+ $result['latitude'] = floatval($exif['GPSLatitude']);
+ $result['longitude'] = floatval($exif['GPSLongitude']);
}
}
}
```
So, long enough for one post ... next post will be about "how to do this better" ;-)
<!-- github-comment-id:314820801 -->
Ok, now, how would I do this with the knowledge I have today (let's see how much time I have at the weekend ;-) )
Use your code for the admin part - that looks promising. Perhaps add some "autodiscovery" for exiftool (try to find it in the path) and try to execute it to see if php allows execution. Depending on that, set $useexiftool and $exiftoolpath in the config
adapt the "is exif support available, otherwise die" part as shown above (this would have to happen in the "original" functions_metadata.inc.php - i.e. as a patch to piwigo)
rely on the OLD code for reading and register a handler for format_exif_data with a VERY HIGH PRIORITY - we probably want to be called before any other handler that merely tries to manipulate the existing data. Also, I would include some kind of "bookmark" in the exif-data that we already read it via exiftool - as with the original code it can happen that format_exif_data gets called twice (and we want to avoid the overhead of reading the file twice via exiftool)
Extend the exif mapping in the conf:
'date_creation' => 'DateTimeOriginal',
// 'rating_score' => 'Rating'
'keywords' => 'Keywords',
'comment' => 'UserComment',
'latitude' => 'GPSLatitude',
'longitude' => 'GPSLongitude'
perhaps add the two lines that do the string=>float conversion ( floatval($exif['GPSLatitude']) ) in the format_exif_data handler - if we have read the data via exiftool
Hope that shows my intentions - lets see what I can do at the weekend!
Ok, now, how would I do this with the knowledge I have today (let's see how much time I have at the weekend ;-) )
1. Use your code for the admin part - that looks promising. Perhaps add some "autodiscovery" for exiftool (try to find it in the path) and try to execute it to see if php allows execution. Depending on that, set $useexiftool and $exiftoolpath in the config
2. adapt the "is exif support available, otherwise die" part as shown above (this would have to happen in the "original" functions_metadata.inc.php - i.e. as a patch to piwigo)
3. rely on the OLD code for reading and register a handler for format_exif_data with a VERY HIGH PRIORITY - we probably want to be called before any other handler that merely tries to manipulate the existing data. Also, I would include some kind of "bookmark" in the exif-data that we already read it via exiftool - as with the original code it can happen that format_exif_data gets called twice (and we want to avoid the overhead of reading the file twice via exiftool)
4. Extend the exif mapping in the conf:
'date_creation' => 'DateTimeOriginal',
// 'rating_score' => 'Rating'
'keywords' => 'Keywords',
'comment' => 'UserComment',
'latitude' => 'GPSLatitude',
'longitude' => 'GPSLongitude'
5. perhaps add the two lines that do the string=>float conversion ( floatval($exif['GPSLatitude']) ) in the format_exif_data handler - if we have read the data via exiftool
Hope that shows my intentions - lets see what I can do at the weekend!
<!-- github-comment-id:314827808 -->
Perfectly fine with me! I got stuck with the plugin when I couldn't figure out how to "overwrite" the normal processing.
BTW, I have made another note where a change might be required (see main.inc.php): "plugins/exif_view/main.inc.php , exif_key_translation (if installed) : return without doing any translations". But I would need to re-visit that - has been a while since I looked into that.
Perfectly fine with me! I got stuck with the plugin when I couldn't figure out how to "overwrite" the normal processing.
BTW, I have made another note where a change might be required (see main.inc.php): "plugins/exif_view/main.inc.php , exif_key_translation (if installed) : return without doing any translations". But I would need to re-visit that - has been a while since I looked into that.
<!-- github-comment-id:314846387 -->
Hi everybody, just a word to tell you that a plugin to read metadata based on exiftool would be a great idea. You already have seen my plugin Exiftool Keywords http://piwigo.org/ext/extension_view.php?eid=848 : yes it's a perfectly "proper" way to overwrite default metadata reading.
What has been lacking for years in Piwigo is a "visual" tool (web form I mean) to configure the mapping between metadata and photo properties in Piwigo. We need something "as simple as possible", not a complex interface as what Advanced Metadata was proposing in the past.
If you need my help, don't hesitate to ask. I will at least review code and test it :-)
Hi everybody, just a word to tell you that a plugin to read metadata based on exiftool would be a great idea. You already have seen my plugin Exiftool Keywords http://piwigo.org/ext/extension_view.php?eid=848 : yes it's a perfectly "proper" way to overwrite default metadata reading.
What has been lacking for years in Piwigo is a "visual" tool (web form I mean) to configure the mapping between metadata and photo properties in Piwigo. We need something "as simple as possible", not a complex interface as what Advanced Metadata was proposing in the past.
If you need my help, don't hesitate to ask. I will at least review code and test it :-)
<!-- github-comment-id:315018324 -->
Not sure how "as simple as possible" could look like. Given the multitude of possible meta data values in an image...
We could limit to the values from $conf['show_exif_fields'] (similarly for iptc). But then you would still need to change this before being able to match meta data to properties. @plegall Do you have anything special in mind? E.g. an app that does it the way you intend?
Not sure how "as simple as possible" could look like. Given the multitude of possible meta data values in an image...
We could limit to the values from ```$conf['show_exif_fields']``` (similarly for iptc). But then you would still need to change this before being able to match meta data to properties. @plegall Do you have anything special in mind? E.g. an app that does it the way you intend?
<!-- github-comment-id:315162149 -->
Hmm, I think "the other way round" could be better.
As far as I know piwigo only STORES a subset of metadata in the database (and only this subset is searchable/groupable etc.) - the rest is only displayed when showing a single photo (and read on the fly from that photo).
So, what about displaying these fixed metadata fields on a webpage and allowing to assign EXIF-Fields from an uploaded demo image to them?
And in a second part allowing to select (other) EXIF fields from the demo picture that just display (= show_exif_fields).
Hmm, I think "the other way round" could be better.
As far as I know piwigo only STORES a subset of metadata in the database (and only this subset is searchable/groupable etc.) - the rest is only displayed when showing a single photo (and read on the fly from that photo).
So, what about displaying these fixed metadata fields on a webpage and allowing to assign EXIF-Fields from an uploaded demo image to them?
And in a second part allowing to select (other) EXIF fields from the demo picture that just display (= show_exif_fields).
<!-- github-comment-id:315329991 -->
Interesting idea @IT-Sumpfling . If I understand well, a user could pick a picture from recent uploads (different pics might have different metadata fields in use), then be shown a table with the following columns filled with every existing field exiftool could read, then use it to set options:
Metadata label
Value
Import in database
Display on photopage
Map to field
Title
Example pic title
boolean
boolean
dropdown list with Piwigo fields, eg. Name
This could also allow to use XMP fields (exiftool reads them) and to select several metadata fields to map to a single Piwigo field, so that pics from different sources with different metadata could be mapped correctly.
Interesting idea @IT-Sumpfling . If I understand well, a user could pick a picture from recent uploads (different pics might have different metadata fields in use), then be shown a table with the following columns filled with every existing field exiftool could read, then use it to set options:
Metadata label | Value | Import in database | Display on photopage | Map to field
------------ | ------------- | ------------- | ------------- | -------------
Title | Example pic title | boolean | boolean | dropdown list with Piwigo fields, eg. Name
This could also allow to use XMP fields (exiftool reads them) and to select several metadata fields to map to a single Piwigo field, so that pics from different sources with different metadata could be mapped correctly.
<!-- github-comment-id:321978341 -->
Just wanted to stress that EXIF/GPS (and not only) reading with PHP7.2 is still buggy (when there are some comments in EXIF fields) and we should find some other way to read it properly.
Mentioned exiftool_keyword (or exiftool_gps) plugin is a workaround, but... exiftool is awfully slow comparing to PHP functions. It has to start full perl script, which is large and complex. Sadly also this Piwigo function 'format_exif_data' is not called only when pictures are uploaded, but also during browsing.
So when I use any of this plugin suddenly my server load goes to 2-3 - while without it is negligible. So I don't think exiftool is the way to go, unless all the metadata are going to be stored in database.
Just wanted to stress that EXIF/GPS (and not only) reading with PHP7.2 is still buggy (when there are some comments in EXIF fields) and we should find some other way to read it properly.
Mentioned exiftool_keyword (or exiftool_gps) plugin is a workaround, but... exiftool is awfully slow comparing to PHP functions. It has to start full perl script, which is large and complex. Sadly also this Piwigo function 'format_exif_data' is not called only when pictures are uploaded, but also during browsing.
So when I use any of this plugin suddenly my server load goes to 2-3 - while without it is negligible. So I don't think exiftool is the way to go, unless all the metadata are going to be stored in database.
<!-- github-comment-id:387839101 -->
Yeah, EXIF reading is buggy in PHP (all versions). Problem I see is: there is no (native) PHP library that really works in all cases. PEL seems better but still buggy for some pictures AND reads the whole picture into memory - this may well also be a performance-killer.
And yes, exiftool is awfully slow and "heavy" compared to native parsing - but I do not think that we will get anything as complete and compatible in PHP.
But of course, depending on the server and the number of users there will be impact - even if we optimize everything to not call exiftool a second time.
This was why I wanted an Admin-UI with the ability to disable exiftool-use (even if present on server).
In a second stage we could use something like https://github.com/romainneutron/PHPExiftool (experimental, still buggy, but could probably be fixed). This starts exiftool a a separate process and uses pipe-mode to feed the pictures. As most PHP-instances are no longer cgi-bin (but instead also "long-running") this could help very much with the startup-times.
Yeah, EXIF reading is buggy in PHP (all versions). Problem I see is: there is no (native) PHP library that really works in all cases. PEL seems better but still buggy for some pictures AND reads the whole picture into memory - this may well also be a performance-killer.
And yes, exiftool is awfully slow and "heavy" compared to native parsing - but I do not think that we will get anything as complete and compatible in PHP.
But of course, depending on the server and the number of users there will be impact - even if we optimize everything to not call exiftool a second time.
This was why I wanted an Admin-UI with the ability to disable exiftool-use (even if present on server).
In a second stage we could use something like https://github.com/romainneutron/PHPExiftool (experimental, still buggy, but could probably be fixed). This starts exiftool a a separate process and uses pipe-mode to feed the pictures. As most PHP-instances are no longer cgi-bin (but instead also "long-running") this could help very much with the startup-times.
<!-- github-comment-id:389203802 -->
Sounds like a plan to me. Unfortunately, the PHPExifTool project hasn't seen any updates in more than 2 years now :-( But maybe there are more active alternatives.
Sounds like a plan to me. Unfortunately, the PHPExifTool project hasn't seen any updates in more than 2 years now :-( But maybe there are more active alternatives.
<!-- github-comment-id:389588503 -->
I still think that we should put all the required metadata in the database along with the image informations. In this case, even slow exiftool would be completely adequate and no need to develop "wheel again". This would be also beneficent for other plugins like OSM plugin, google map plugin and other that stores some parts of metadata on their own.
We would have also one standard (for piwigo) scheme for image metadata (no metter if it started with Exif, IPTC, XMP) - so no more "translation" of metadata. And finally we could do export of those metadata with images (if we would changed them in piwigo only - like comment, author, GPS position) for upgrades, migration, archiving, whatever...
Currently generating, with exiftool, single html image page (I have metadata exposed there) takes 4s (yes 4 seconds) in my case. With php build in exif functions about 200ms.
I still think that we should put all the required metadata in the database along with the image informations. In this case, even slow exiftool would be completely adequate and no need to develop "wheel again". This would be also beneficent for other plugins like OSM plugin, google map plugin and other that stores some parts of metadata on their own.
We would have also one standard (for piwigo) scheme for image metadata (no metter if it started with Exif, IPTC, XMP) - so no more "translation" of metadata. And finally we could do export of those metadata with images (if we would changed them in piwigo only - like comment, author, GPS position) for upgrades, migration, archiving, whatever...
Currently generating, with exiftool, single html image page (I have metadata exposed there) takes 4s (yes 4 seconds) in my case. With php build in exif functions about 200ms.
<!-- github-comment-id:390469284 -->
Yesterday I was discussing with @EddyLB and I realized that some hosting providers still disable the exec PHP function. Making it impossible to use ffmpeg, but also exiftool. Even if we rely quite a lot on exec, we need to decide if it's really a prerequisite or not to run Piwigo.
(I don't forget this issue)
Yesterday I was discussing with @EddyLB and I realized that some hosting providers still disable the `exec` PHP function. Making it impossible to use `ffmpeg`, but also `exiftool`. Even if we rely quite a lot on `exec`, we need to decide if it's really a prerequisite or not to run Piwigo.
<!-- github-comment-id:2222273126 -->
@plegall , in situations where the server cannot use exec() or the user cannot install FFmpeg or ExifTool, a workaround could be that third party apps like the iOS and Android apps, which are more and more used to take photos/videos, provide Exif metadata, thumbnails, etc. after uploading photos.
After a successful upload, pwg.images.uploadAsync may return data requesting the app to upload thumbnails of given sizes and/or metadata.
@plegall , in situations where the server cannot use exec() or the user cannot install `FFmpeg` or `ExifTool`, a workaround could be that third party apps like the iOS and Android apps, which are more and more used to take photos/videos, provide Exif metadata, thumbnails, etc. after uploading photos.
After a successful upload, `pwg.images.uploadAsync` may return data requesting the app to upload thumbnails of given sizes and/or metadata.
<!-- github-comment-id:2226841606 -->
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
PHP native EXIF parser has a lot of issues, and sometimes is not even available.
We should replace
exif_read_databy a 3rd party and autonomous library (do not rely on external binaries like exiftool).I found https://github.com/lsolesen/pel (using namespaces, perhaps it's time for Piwigo to switch to PHP 5.3...).
Perhaps there are more.
Need to do a benchmark before making a choice.
(related to a very old ticket #338)
That's definitely possible. I'm perfectly OK with that. 5.4 could easily become a requirement now.
I don't know if PEL is the best way to go, but using a better exif parser could soon become an "important task". Several recent issue on GPS parsing, when makernotes are in the EXIF metadata.
Hi, I do not want to interfere, but it seems to me that all Exif libraries for PHP are no longer maintained (last commit 2013 or even 2011).
On the other hand, the "de-facto-standard" in Exif parsing (Exiftool) is really a Perl-library http://www.sno.phy.queensu.ca/~phil/exiftool/
So, if you do not want to call an external binary and probably do not want to maintain a Exif parsing library in PHP yourself:
Wouldn't it be possible to use the exiftool-library via PECL ?
https://pecl.php.net/package/perl
On the other hand it would be possible (and in fact I have already done so on my own server) to use exiftool if present and fall back to the php implementation if not - these changes are just a few lines of code in 2 or 3 files ...
The main target of Piwigo is still a majority of shared hosts with limited capabilities in term of extensions and binaries.
But the lib in my first message was updated 7 months ago, not in 2013...
I stand corrected - it seems that isolesen re-started developing and fixing bugs in his library (or never stopped and just didn't update the old homepage).
And I perfectly understand the primary target of piwigo - this is why I tried to think of ways to include exiftool (which I know works very well for all kinds of images (even videos and with severely "broken" metadata) ) without breaking the possibility of shared hosting. As I am not "fluent" in PHP I took a look at the details of PECL and PECL::perl yesterday and have to agree - this is probably not for shared hosting (at least not when there is not even a PHP::exif available ...)
Of course a stable and good working library in "native PHP" would be the best solution. If PEL is / will be that library - great! I definitely look forward to having reliable Exif-Support without my "personal patches" (i.e. using exiftool in the background).
Is there anything happening with this idea? Just curious...
Hmm, not as far as I can tell. Perhaps I will cleanup and submit my "exiftool-Patches".
They use exiftool if present and fall back to the default behaviour when not ... so, only improvements there ;-)
One thing about PEL that I don't like is that it cannot parse metadata for movies - and as far as I can see stores the whole image in memory for parsing - that could be a problem for smaller virtual hosting packages ...
Of course, "using exiftool if present, fall back to PEL if not" would be best - and circumvent the broken inbuilt PHP exif support ...
@IT-Sumpfling Please do so! I did hack a bit on my end as well so we should be able to come up with a decent support for exiftool (I created a small plugin to maintain exiftool path and parameters...).
check out http://piwigo.org/ext/extension_view.php?eid=850 and http://piwigo.org/ext/extension_view.php?eid=848 It may help to solve your issues and to build a proper fix if needed
The extensions look interesting - is that the "proper" way to override the builtin exif parsing (can it also the "default" properties like date etc.)?
I will send the exiftool-commandline I use - which "automatically" formats the different values like the builtin ... and uses php output format - so no need to do json-parsing.
Who would have guessed it could be so easy??? @IT-Sumpfling If you're interested you might have a look at https://github.com/ThomasDaheim/piwigo-stuff/tree/master/piwigo-exiftool - my attempt to write a complete plugin that allows to customize exiftool path & parameters. Combined with your code to call exiftool that would be all we need.
@ThomasDaheim
Sorry about the delay, here my "patch" for functions_metadata.inc.php
Github doesn't allow me to upload it, so I'll paste it as text.
Comments between the single blocks ...
This would be replaced by your config. I use a boolean variable if exiftool is really available because some other checks depend on this. This check is probably better in the config, too.
The old version stops here if no php exif extensions are available - as we try to be independent of these, that would not be helpful ...
I don't particularly like this part - neither the original nor my changes. but it was the best I could come up with in a hurry which should work in any case and keep close to the original.
What the original did was:
Honestly I do think that we could get away with calling "format_exif_data" only once - if read_exif_data fails, $exif is empty anyway. But important: here is the "magic line" to call exiftool:
eval('$exif=' .
exiftool -php -c "%+.14f" -s -sep "," -q $filename);will call exiftool in PHP-mode ( -php returns the result as PHP array), formatting GPS-coordinates as floats in the same format as used by piwigo (-c "%+.14f") use "," as list seperator (-sep ",") and use short output format (-s) and be quiet (-q).
Store the output in $exif and evaluate it - bingo - everything is there!
The last part deals with GPS-coordinate parsing:
My patch uses the old code when exiftool is not used (if ! $useexiftool) and just assigns the floats when exiftool is used:
So, long enough for one post ... next post will be about "how to do this better" ;-)
Ok, now, how would I do this with the knowledge I have today (let's see how much time I have at the weekend ;-) )
'date_creation' => 'DateTimeOriginal',
// 'rating_score' => 'Rating'
'keywords' => 'Keywords',
'comment' => 'UserComment',
'latitude' => 'GPSLatitude',
'longitude' => 'GPSLongitude'
Hope that shows my intentions - lets see what I can do at the weekend!
Perfectly fine with me! I got stuck with the plugin when I couldn't figure out how to "overwrite" the normal processing.
BTW, I have made another note where a change might be required (see main.inc.php): "plugins/exif_view/main.inc.php , exif_key_translation (if installed) : return without doing any translations". But I would need to re-visit that - has been a while since I looked into that.
Hi everybody, just a word to tell you that a plugin to read metadata based on exiftool would be a great idea. You already have seen my plugin Exiftool Keywords http://piwigo.org/ext/extension_view.php?eid=848 : yes it's a perfectly "proper" way to overwrite default metadata reading.
What has been lacking for years in Piwigo is a "visual" tool (web form I mean) to configure the mapping between metadata and photo properties in Piwigo. We need something "as simple as possible", not a complex interface as what Advanced Metadata was proposing in the past.
If you need my help, don't hesitate to ask. I will at least review code and test it :-)
Not sure how "as simple as possible" could look like. Given the multitude of possible meta data values in an image...
We could limit to the values from
$conf['show_exif_fields'](similarly for iptc). But then you would still need to change this before being able to match meta data to properties. @plegall Do you have anything special in mind? E.g. an app that does it the way you intend?Hmm, I think "the other way round" could be better.
As far as I know piwigo only STORES a subset of metadata in the database (and only this subset is searchable/groupable etc.) - the rest is only displayed when showing a single photo (and read on the fly from that photo).
So, what about displaying these fixed metadata fields on a webpage and allowing to assign EXIF-Fields from an uploaded demo image to them?
And in a second part allowing to select (other) EXIF fields from the demo picture that just display (= show_exif_fields).
Interesting idea @IT-Sumpfling . If I understand well, a user could pick a picture from recent uploads (different pics might have different metadata fields in use), then be shown a table with the following columns filled with every existing field exiftool could read, then use it to set options:
This could also allow to use XMP fields (exiftool reads them) and to select several metadata fields to map to a single Piwigo field, so that pics from different sources with different metadata could be mapped correctly.
Just wanted to stress that EXIF/GPS (and not only) reading with PHP7.2 is still buggy (when there are some comments in EXIF fields) and we should find some other way to read it properly.
Mentioned exiftool_keyword (or exiftool_gps) plugin is a workaround, but... exiftool is awfully slow comparing to PHP functions. It has to start full perl script, which is large and complex. Sadly also this Piwigo function 'format_exif_data' is not called only when pictures are uploaded, but also during browsing.
So when I use any of this plugin suddenly my server load goes to 2-3 - while without it is negligible. So I don't think exiftool is the way to go, unless all the metadata are going to be stored in database.
Yeah, EXIF reading is buggy in PHP (all versions). Problem I see is: there is no (native) PHP library that really works in all cases. PEL seems better but still buggy for some pictures AND reads the whole picture into memory - this may well also be a performance-killer.
And yes, exiftool is awfully slow and "heavy" compared to native parsing - but I do not think that we will get anything as complete and compatible in PHP.
But of course, depending on the server and the number of users there will be impact - even if we optimize everything to not call exiftool a second time.
This was why I wanted an Admin-UI with the ability to disable exiftool-use (even if present on server).
In a second stage we could use something like https://github.com/romainneutron/PHPExiftool (experimental, still buggy, but could probably be fixed). This starts exiftool a a separate process and uses pipe-mode to feed the pictures. As most PHP-instances are no longer cgi-bin (but instead also "long-running") this could help very much with the startup-times.
Sounds like a plan to me. Unfortunately, the PHPExifTool project hasn't seen any updates in more than 2 years now :-( But maybe there are more active alternatives.
I still think that we should put all the required metadata in the database along with the image informations. In this case, even slow exiftool would be completely adequate and no need to develop "wheel again". This would be also beneficent for other plugins like OSM plugin, google map plugin and other that stores some parts of metadata on their own.
We would have also one standard (for piwigo) scheme for image metadata (no metter if it started with Exif, IPTC, XMP) - so no more "translation" of metadata. And finally we could do export of those metadata with images (if we would changed them in piwigo only - like comment, author, GPS position) for upgrades, migration, archiving, whatever...
Currently generating, with exiftool, single html image page (I have metadata exposed there) takes 4s (yes 4 seconds) in my case. With php build in exif functions about 200ms.
(I don't forget this issue)
Yesterday I was discussing with @EddyLB and I realized that some hosting providers still disable the
execPHP function. Making it impossible to useffmpeg, but alsoexiftool. Even if we rely quite a lot onexec, we need to decide if it's really a prerequisite or not to run Piwigo.@plegall , in situations where the server cannot use exec() or the user cannot install
FFmpegorExifTool, a workaround could be that third party apps like the iOS and Android apps, which are more and more used to take photos/videos, provide Exif metadata, thumbnails, etc. after uploading photos.After a successful upload,
pwg.images.uploadAsyncmay return data requesting the app to upload thumbnails of given sizes and/or metadata.