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

Support Immich albums as media source #163

Open
bugfest opened this issue Aug 27, 2024 · 26 comments · Fixed by #164
Open

Support Immich albums as media source #163

bugfest opened this issue Aug 27, 2024 · 26 comments · Fixed by #164
Assignees
Labels
enhancement New feature or request

Comments

@bugfest
Copy link
Contributor

bugfest commented Aug 27, 2024

Immich (https://immich.app) is a popular self-hosted media management server. It provides a very well documented API over HTTP.

This media source would let AerialViews to access an Immich album (pictures and videos) including these assets metadata (e.g: file-type, description, location).

PS. I've prepared an implementation for your consideration. I'll open a PR

@bugfest bugfest added the enhancement New feature or request label Aug 27, 2024
@theothernt
Copy link
Owner

theothernt commented Aug 29, 2024

Thanks for the PR, I've only had a quick look but it looks good so far. My plan would be to put this into the v1.7.3 as v1.7.2 is nearly ready.

Part of the reason is that as I add more 'providers', I thought I could detect them via the URL (http:// vs smb:// vs file://) but that doesn't seem to work well. So I will add a type/enum to each provider so it's easier to parse for playback in either ImagePlayer or VideoPlayer.

Also translations, which are done by the community, can take a week or two.

@bugfest
Copy link
Contributor Author

bugfest commented Aug 29, 2024

That sounds fantastic. Do you have a tentative date for the release so that I can start migrating this implementation?

@theothernt
Copy link
Owner

I hope to release v1.7.2 next week. There will be a beta tomorrow for a few users to test some requested features - if no issues are reported, then it'll be released.

Also, I will add that type/enum piece tomorrow.

@bugfest
Copy link
Contributor Author

bugfest commented Aug 29, 2024

Great. I'll move #164 to draft in the mean time!

@RyanShahidi
Copy link

If its of interest I made a fork that also supports immich. I really only made it for my personal use and only got to the point to get images working but its been working well enough on a shield: https://github.com/RyanShahidi/AerialViewsImmich

@theothernt
Copy link
Owner

v1.7.2 has been out a little while and seems stable, so we can continue with the merge of this PR.

I have a few small changes to suggest, so I'll add those now.

@theothernt
Copy link
Owner

Ok, that's merged and I made a few small changes.

@RyanShahidi do you want to give this version a try - I can make a beta/release version if you like?

@RyanShahidi
Copy link

I can check it out by this weekend and no need to make the release version, I can grab the latest code! I looked at it briefly, would there be any interest in modifying the log-in to utilize an API key instead of password with auto-fetching of the albums? I think that would be a bit more user friendly and I can work on implementing that too if there would be interest. It would be similar to what I had previously done so shouldnt be too hard to combine, just let me know.

Just in-case that wasnt super clear, here is the current implementation login
362006428-4c86d9b9-f26e-4c3a-8b06-f2769c06d6e4

And here is what I previously had done
image

@theothernt
Copy link
Owner

Oh, I see - an API key + server URL would be a easier then. I'd accept a PR for that.

And although I don't use Immich, I see they have a demo instance that I can use for testing in future.

@theothernt
Copy link
Owner

Oh, I see - an API key + server URL would be a easier then. I'd accept a PR for that.

I've been thinking about this - the API gives full access to all images, etc but "Shared links" allow you to share specific albums or photos - so would that be better for some people?

@RyanShahidi
Copy link

Thats a fair thought because your point is true. Personally, I still feel like the API might be the better route because the API key is shorter to enter, would allow more easily switching between albums, and is less likely to have user errors (for example, they forget to put /share/ as part of the url). The risk might also be pretty minimal as I would imagine the vast majority of users would be using this to stream an album from a computer in their home to the android tv in their home.

A similar sort of app, Immich Kiosk utilizes the API instead of shared albums so its not completely unprecedented. These are all just my opinions though, let me know your thoughts because Id have no problem working on it if its of interest.

@theothernt
Copy link
Owner

Ok, once you have an API key - how do you choose an album?

@theothernt theothernt reopened this Oct 4, 2024
@bugfest
Copy link
Contributor Author

bugfest commented Oct 5, 2024

I guess that would require a new fragment to populate and show the user a list of available albums isn't it @RyanShahidi ?

@RyanShahidi
Copy link

Yeah exactly, how I worked it in is after the API key is entered, when you press the select album button it makes a request to the API to get all available albums and then the user would just click whichever one they want.

@theothernt
Copy link
Owner

There will be an issue with trying to display a list in a new fragment. I currently use the Preference UI, which is old but it works on TVs and phones, unlike most of the Android UI components. I don't think it can display a long list, so the only option might be a ListPreference.

As for API access vs Shared album access, we could add support for both. For local file access, I have to support MediaStore and legacy direct folder access. I just need to think about what details are needed from the user and the best way to present the two UIs...

Common: scheme / hostname / port
Shared Links: share link / password
API Access: api key / way to pick album

@RyanShahidi
Copy link

I briefly discussed this in the pull request, but I think I came up with an OK solution to have both the API access and shared links access. I commented on it there, but when using the API access videos will not load. I couldnt figure it out but might have some time over the week to work on it too. I am fairly certain its just an authentication issue with passing the API key in a header, but I am not certain how to fix that or if it is the only issue.

Any help would be greatly appreciated!

@theothernt
Copy link
Owner

Thanks @RyanShahidi for the PR, I just had a quick look at it.

My basic plan is... I want to get this release finished up in the next couple of days, so I'll leave the Immich provider hidden in the menu.

I just setup a small instance of Immich on my home server, so I'll have a play around with the code in a couple of days. I'll be testing with a few photos and videos of different aspect ratios - I also want to fix the bugs with fill/scale on TV and phone.

I also need to refactor VideoPlayerView and ImagePlayerView.

@RyanShahidi
Copy link

RyanShahidi commented Oct 7, 2024

Definitely agree on keeping it hidden, it needs a bit of polish. Some other minor things I want to fix, in addition to having videos work (just so there is a list so we are all aware):

  • Selected Album with the API key should display album name
  • Test Connection Setting should be a bit more robust
  • Shared link should have some checks to fix any incorrect URLs (eg: not having forward slashes in the right spot)
  • Choose media type does not appear to have any effect
  • Evaluation of better method to handle SSL

@RyanShahidi
Copy link

I think I fixed most of the issues, @bugfest would you be able to test it as well? I am testing the current immich implementation on a shield and I have not run into any issues, with both API and shared links working, giving appropriate error messages, and displaying both images and videos.

I did try a slightly more secure method of handling certificates, but I think it should hopefully be updated to no longer implement a custom X509TrustManager. I can keep looking into it but if anyone else has a better idea that should be implemented. Let me know if there are any comments/suggestions!

@theothernt
Copy link
Owner

I should have some time over the next few days to look at the code. The cert. related code for OkHttp, is that for regular SSL or custom certs?

@RyanShahidi
Copy link

So sorry, been crazy busy! For the most part it will work with both. In one of the later commits I added code to first try standard SSL with default trust manager, then to fall back to a permissive mode for custom certs if that fails. While obviously not ideal, it seemed a bit more secure than just blindly trusting everything.

@bugfest
Copy link
Contributor Author

bugfest commented Nov 1, 2024

Hi guys, sorry I've been out for a while. @RyanShahidi I plan to run those tests in my devices during the following days. I'll let you know if I spot any issue. Very nice progress!

Regarding SSL certificates, IMHO we should have an specific option to ignore the certificate validation and by default we should trust only certificates in the device's trust store.

@theothernt
Copy link
Owner

No problem at all - both of you - we all have lives and things to do 👍🏻

I'll try and make time over the next few days to do some testing as well, although for the moment I can only test HTTP with Immich running in a docker instance.

Agreed with @bugfest about the custom certs stuff - so the dropdown could have HTTP, HTTP, Custom Cert. ?

@bugfest
Copy link
Contributor Author

bugfest commented Nov 1, 2024

Hi @theothernt

so the dropdown could have HTTP, HTTP, Custom Cert. ?

I think we could achieve this with just two fields: URL and Validate SSL certificate

  • URL - string (e.g.: https://myserver:port/base/path/) if the user does not include protocol we can assume it's "http://". Examples (input and calculated value):
Input Calculated URL value
192.168.8.4 http://192.168.8.4
192.168.8.4:8080 http://192.168.8.4:8080
http://192.168.8.4:8080 http://192.168.8.4:8080
https://192.168.8.4:8443 https://192.168.8.4:8443
https://192.168.8.4/base/path https://192.168.8.4/base/path
  • Validate SSL certificate - boolean (enabled by default)

@RyanShahidi
Copy link

I agree I think that is a great way to handle it! I might have some time next weekend to get working on it too

@RyanShahidi
Copy link

I believe the SSL certificate toggle is appropriately implemented like @bugfest recommended. So far, I have verified that it is working for both images and videos but obviously if you guys are able to test it too that would be preferred. It required a bit of reworking so I might have overlooked something. Also, because this was the last major hurdle I updated the pull request to ready for review. Let me know if there are any issues, I would be more than happy to help!

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

Successfully merging a pull request may close this issue.

3 participants