jmathai / elodie

An EXIF-based photo assistant, organizer and workflow automation tool.
https://bit.ly/introducing-elodie
Apache License 2.0
1.26k stars 139 forks source link

[Feature Request] Extend Plugin mechanism #337

Open rkr-kununu opened 5 years ago

rkr-kununu commented 5 years ago

So, I've been reviewing the current plugin mechanism. It looks promising, but I'd like to discuss extending it.

My rational is that I've got a bunch of custom use-cases that probably don't fit into the Elodie. Currently, I have my own fork which I've kludged with the features I want. For example, I've got a bunch of old mpegs but I've been re-encoding them so they are suitable for streaming via a web browser.

Anyway, my suggestion would be in extending PluginBase to include:

  # Determine if 'file_path' should be imported or not. An ElodiePluginError() can be raised during internal errors.
  #  Parameters: 'file_path' will be a string referring to the location of the file to import (or not)
  #  Return:  True this plugin believes the file should be imported, False this plugin believes that this file has already been imported.
  def import(self, file_path):
  # This plugin would be called in-between 'before()' and 'after()'.  This permits plugins to stage changes to the EXIF information _before_ it's written to disk.
  #  Parameters: media will be type 'Media:Base'
  #  Return:  The return value is ignored, but upon an error an ElodiePluginError() can be raised.
  def during(self, media):

Of course, we'd need to create:

  # This will loop through all the 'import' plugins.  However, if any plugin returns False - this function will return False.
  def run_all_import(...)
  def run_all_during(...)

Ideally, I'd like during() to simply modify an in-memory representation of the EXIF information. I've noticed that PyExifTool::set_tags() does permit a dictionary of EXIF fields to be set. However, the way that Elodie currently behaves, each time Media::__set_tags() is called this shells-out a call to ExifTool. This means we are re-writing the whole image/video to disk each time a call is made to either Media::set_album(), Media::set_original_name(), Media::set_title(), etc. Elodie does not currently easily allow us handle "multiple" changes simultaneously (ie: set the original name and the creation date) as we're also stuck try to juggle around the '_original' file that ExifTool creates. It's also difficult for Elodie to remove EXIF information. However, with an in-memory representation of the EXIF information, we could simply make a "copy" of the in-memory Media object, feed it though the plugins, and perform a diff between the "original" in-memory Media object and compare that with the "copy" that was passed into the plugins.

By having a series of plugins, we could also move a bunch of Elodie's business logic into a varity of plugins.

  1. The creation/modification of the 'hash.json': This would be a single plugin defining 'import()' (to check if the file already was imported) and 'after()' (to write the sha256 to the hash.json)
  2. The extraction of the original_name (if not set), by defining the 'during()' function.
  3. The geolocalization (this would allow the user to choose either MapQuest Plugin and/or Google Maps Plugin), by defining the 'during()' function.
  4. etc

I think this will also clean-up a bunch of Elodie's code:

  1. All the cache invalidation would go away (as we'd be writing to disk once), so Media::Base::reset_cache() would go away.
  2. Unit testing should be much easier, as we'd only need to write asserts on the in-memory Media object (instead of actually writing them to disk).
  3. It should be easier for the open-source community to contribute to Elodie as we're talking about creating/modifying plugins (knowledge of the underlying code base is not required).

Anyway, before anyone invests a bunch of effort in this - I would like to know if these changes are inline with how you view Elodie could work? And what could be done to improve this workflow.

jmathai commented 5 years ago

Hi @rkr-kununu,

Yes --- extending the plugin framework is inline with how I'd like to add functionality which doesn't belong in the core code. I'm not sure how much of the existing core should be moved out to a plugin since it works and has been tested for a few years though.

But new features should be evaluated against core or plugin.

I think there's promise in passing around a copy of the entire EXIF data and modifying that and writing it all back to the image in one shot. I haven't tried or tested exactly how that would work but it seems like an improvement, generally speaking. It may also make it easier to enable plugins to modify EXIF.

I suggest starting with a single use case like you described and extending the plugin framework as part of a series of pull requests (one for the plugin framework and another for the functionality you're trying to add).

Thanks for the suggestion.