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

Get original files #166

Merged
merged 3 commits into from
Jan 5, 2025
Merged

Get original files #166

merged 3 commits into from
Jan 5, 2025

Conversation

pe1uca
Copy link
Contributor

@pe1uca pe1uca commented Dec 10, 2024

I'm not so sure if this is a good approach as there's no straight forward way to get the full path of the original image.

The data has a files array, which has objects with both index and filename, so I used them to construct a unique path for each image.
On the server side, there's no direct way to get the index value of the entry's file property, since the sources property of the config has an array with objects which index property has something like /data/config/Pictures.idx, so this PR assumes the name of the idx file is the same value as the one in the entry.

This PR relates to #115, #159, and #165 (although probably there's more to do to exactly implement the feature of each issue)

@xemle
Copy link
Owner

xemle commented Dec 10, 2024

Hi @pe1uca

thank you very much for your PR, our time and the ability to offer the original files.

While your PR is excellent and functional correct, I do not like to expose my original files by default. The design decision was not to deal with original files at the beginning.

So I would like to discuss possible solutions before applying your PR since it clashes with my original thoughts of the gallery and I need some time to convince myself to add such feature. So apologize the slowdown of this issue.

Please have a look at internals to get a basic understanding of the design decisions, architecture and functional building blocks of the gallery.

Home Gallery was meant to be a lightweight overlay for my media files, while "protecting" my original files. The idea was to provide a gallery view of my 100.000 files with 1.5 TB original size on a thumb drive via a tiny device like a Raspberry Pi while my original files are not available/accessible on the gallery server device.

I understand the requirement of providing original files and you referred to related issues. That is great. And it is awesome to receive your time and work. So let's try to get both world together - the original architecture and the demand of the community.

Two parts hinders me to apply your PR right away:

  • Offline indices are not covered. Offline indices are sources which indices are not updated. The extractor could run and could extract new things from given files in the storage but the original file sources are not available. E.g. by an disconnected external backup HDD

  • Explicit option to enable original files by server, webapp and export would be awesome. The initial design decision was not to expose original files. So the gallery can be hosted on any device. This should be implemented as opt-in feature

One of my idea dealing with original is to offer the original files in the storage directory. The current storage API for the extractor offers a copy or symlink option to include the original files in the storage. This would enable the original idea that you can copy the storage directory to another device (e.g. via fetch command), including the original files.

While the extractor, database mapper and the query are customizable by plugins API the view plugin exists only as POC on my disk. Nor a express middleware plugin API exists to support your solution as plugin. So some parts of the original file feature could be extended by the plugin API already without changing the core.

I believe to open the gallery via plugin API is a great idea to customize the gallery. The plugin API lacks of some parts to completely implement your feature as plugin. But in the meanwhile the view could be hard coded but the rest could be implemented as extractor, database mapper and query plugin.

So my proposal is that the feature is implemented as plugin: The extractor could symlink or copy the original files to the storage, the database mapper adds the original files to the preview array, the query plugin extends the query language to search for original files and the webapp is hardcoded until the view is supported by the plugin API.

What do you think? We could have also a 1-2-1 to discuss this issue in a call...

@pe1uca
Copy link
Contributor Author

pe1uca commented Dec 11, 2024

For sure we can have a call to get a better sense on how to properly implement this!

The extractor could symlink or copy the original files to the storage, the database mapper adds the original files to the preview array, the query plugin extends the query language to search for original files and the webapp is hardcoded until the view is supported by the plugin API.

Are we sure this isn't more complicated than what is worth for this feature?

I have a few comments/questions going the plugin route:

  • For the extractor, I'd say using a symlink would be better, it'd still allow access to the original files, but devices with low resources could still be used since there won't be a need to copy all the original files.
  • Why add the files to the previews instead of adding them to the files array? I mean, since it's the original file, not a preview and the rest of the code can assume previews are always available, and the files can still be dependent on the offline flag for the source (specially while using symlinks).
  • I'm not sure I understand the need to extend the query language. Wouldn't this make the users go into the search view instead of staying in the view of the image? I'd guess user want to see the original file in the same view they right now use to see a single image instead of having to jump into the search and then into another single image view.

In case the option in this PR is good enough, to make it opt-in an option to the source could be added, something like serveOriginalFiles: true
offline could take priority over this new setting.
Still, probably this will need an update to some part of the code to store the original full path of the file and make it available to the UI to request (not sure if the parsing of the index currently implemented is enough)

@xemle
Copy link
Owner

xemle commented Dec 13, 2024

Hi @pe1uca

thank you for your reply.

I had again some thoughts regarding your PR, your reply and I like the simplicity of your provided solutions. It helps me to think out of the box of my initial intentions and assumptions.

pros are

  • Add a download link to the file
  • Reuse the source directories as static middleware
  • Minimal change set

cons are

  • Not configurable
  • Enabled by default

What about following addition:

  • Add a flag e.g. downloadable to a source definition
  • Configure the middleware/filter sources by that flag
  • Add the flag to the webapp config on the server
  • The view adds a download link depending the the webapp config

The middleware should be /sources/<indexName>/<filePath> or even /files/<indexName>/<filePath> or /files/sources/<indexName>/<filePath>.

Further the middleware could be extended later that only specific uses can download files from sources which will filter the webapp config, too. So the download feature becomes configurable.

The static export could be extended later, too to add original files.

Then I am happy :-)

What do you think?

Add opt-in option per source in configuration.
Change URL of original files.
Improve getting name of index.
Only render link to download based on source configuration.
@xemle
Copy link
Owner

xemle commented Dec 25, 2024

Thank you @pe1uca for your update. Your change looks promising and I hope can merge it in the next days. Please poke me if there is any delay

@xemle
Copy link
Owner

xemle commented Jan 2, 2025

Hi @pe1uca I've made some changes into your work. Please have a look: master...feature/webapp-download-original-files

Basically, I've moved the download link to the file detail section because a media can have multiple files: Raw, JPG and xmp sidecar file. Each of such file can be downloaded separately. What do you think?

@xemle xemle merged commit f1c58e3 into xemle:master Jan 5, 2025
1 check passed
@xemle
Copy link
Owner

xemle commented Jan 5, 2025

@pe1uca I've updated your work and moved the rest endpoint to /api/sources. /api/sources exposes also a list of downloadable indices.

Thank you again for your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants