Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion on supporting IPTC2 tags #49

Open
FrivalszkyP opened this issue Feb 17, 2016 · 5 comments
Open

Suggestion on supporting IPTC2 tags #49

FrivalszkyP opened this issue Feb 17, 2016 · 5 comments
Labels

Comments

@FrivalszkyP
Copy link
Contributor

Thanks for yesterday's quick review of my pull request!

I have a suggestion for supporting some of the IPTC2 tags. As of right now, I'm doing this by extending your \PHPExif\Mapper\Exiftool class and adding just a couple of lines of code. But this could be done more neatly I believe. Here's the code I'm using (I've omitted my namespace declaration for code copyright reasons):

use \PHPExif\Exif;
use \DateTime;

/**
 * PHP Exif Exiftool Mapper
 *
 * Maps Exiftool raw data to valid data for the \PHPExif\Exif class
 *
 * @category    PHPExif
 * @package     Mapper
 */
class Mapper extends \PHPExif\Mapper\Exiftool
{
    const CREDIT2                  = 'IPTC2:Credit';
    const BYLINE                   = 'IPTC:By-line';
    function __construct() {
        $this->map[ self::CREDIT2 ] = Exif::CREDIT;
        $this->map[ self::BYLINE ]  = Exif::AUTHOR;
    }
}

I do not want to push this and submit it as a pull request as my version simply overwrites the original Exif::CREDIT and Exif::AUTHOR values. It suits my needs, but it might not be the best idea for general purposes.

I'm attaching an image that is suitable for testing. (It's the same image I submitted for yesterday's pull request, but I have more examples if needed).

europress-getty1

@Miljar
Copy link
Collaborator

Miljar commented Feb 17, 2016

Hi Péter,

Thanks for your suggestion. At this moment, there is not really a solution for allowing both the IPTC & IPTC2 data. I'm aware of this problem (besides other problems of course) but I don't see an option right now to update this code.

However, some time ago, I have started rewriting this library. One of the changes is that I've now split up Exif data & IPTC data in separate data classes. This opens up the possibility to add an IPTC2 data class.

Currently, I'm lacking the extra time to really push this rewrite forward and none of that code is on Github yet. If you're interested, I can try to get this on Github as soon as possible so you can have a look. It's very early in the rewriting process though, so things can change rapidly. If you're up for it, any help would be appreciated too, but don't feel any obligation for that.

@FrivalszkyP
Copy link
Contributor Author

Sure thing Miljar, thanks. Your solution sounds promising. If you could fork your own code maybe or just create a dev branch, I'd like to check it out! Thanks for the heads-up!

@Miljar
Copy link
Collaborator

Miljar commented Feb 17, 2016

It's up in the feature/rewrite branch. I should warn you, it's very much a work in progress :)

@FrivalszkyP
Copy link
Contributor Author

Noted, thanks!

@Miljar Miljar added this to the v1.0 milestone Feb 17, 2016
@Miljar
Copy link
Collaborator

Miljar commented Mar 21, 2016

Moving forward with the rewrite, all new code is being pushed to the develop branch instead, since would like to follow github-flow.

@Miljar Miljar removed this from the v1.0 milestone Dec 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants