-
Notifications
You must be signed in to change notification settings - Fork 94
Add usage of exiftool to get exif tags from camera manufactures #189
Conversation
…lens info Signed-off-by: Michael Rasmussen <[email protected]>
Signed-off-by: Michael Rasmussen <[email protected]>
Can you also add this PR to https://github.com/LycheeOrg/Lychee-laravel ? And also make it such that it is the user who decides whether to use the php version of the system version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice PR. I'm not really eager to add system calls but if this is something the user can willingly enable then I'm okay with it.
Signed-off-by: Michael Rasmussen <[email protected]>
I have made the required changes. See next PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. You might want to ditch the public static function setUseExiftool($enable) {
function as this implies to also change the front end (Lychee-front).
A use can modify this value just by going to the advanced settings (and click "yes I understand the risks").
A database update file is required.
As can be seen I have prepared the code in such way that using exiftool can be changed on demand by the user when opening more in the settings dialog. Since using external tools is potential dangerous I have added a notice logging when using exiftool. Only thing missing is a database update script to add the new column 'useExiftool' to the database with the default configuration '0'. |
I appreciate that. Good idea. |
Signed-off-by: Michael Rasmussen <[email protected]>
Signed-off-by: Michael Rasmussen <[email protected]>
Signed-off-by: Michael Rasmussen <[email protected]>
Thanks a lot. |
Is it enough to make changes to this file 'Lychee-Laravel/app/Metadata/Extractor.php and 'Lychee-Laravel/app/Configs.php'? |
For the database you can do
This will create a migration file in For the content, you can have a look at: |
Hi,
[edit: Code block to kill the auto issue references (d7415)] |
Can you please remove that last commit. This is unrelated to the current PR. Additionally, this is already provided by: |
(instructions at #134) |
This is the main issue with using master for PRs. I'm pretty sure you can't change branch without recreating the PR either 😞 |
How can I push to my own branch again with out poluting here? |
This comment has been minimized.
This comment has been minimized.
If you want to develop things without risking to pollute your master, the easiest way is to : git checkout -b name_of_your_new_branch
git add # what ever you want to add
git commit # and your commit
git push origin name_of_your_new_branch Before making a PR, it is usually a good idea to pull what is on the master and rebase your branch on it. |
|
Only in the changelog and in that Issue. Reasoning being that it should only be required after an upgrade from before the |
Using exiftool for getting exif tags make available a lot more tags than the built-in functionality in PHP. Using exiftool will make eg. lens info available without having to rely on that users have exported a raw from Ligthroom.
Signed-off-by: Michael Rasmussen [email protected]