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

Add Emby Support #365

Merged
merged 6 commits into from
Feb 4, 2024
Merged

Add Emby Support #365

merged 6 commits into from
Feb 4, 2024

Conversation

bpwhelan
Copy link
Contributor

@bpwhelan bpwhelan commented Jan 30, 2024

I'm expecting a bit of pushback, but I'm mostly happy with this as of right now.

  • Couldn't figure out a config, made the "isVideoPlayerPage()" logic quite a bit more complicated, the host regex is overly simple as is...
  • path doesn't work since emby uses the same path for everything
  • Might be able to just go through ApiClient Directly? But i already had the code written to hit the RESTApi directly.

examples of urls where we notice the video player and attempt to get subtitles.

http://ip:8096/web/index.html#!/videoosd/videoosd.html

https://emby.custom.url/web/index.html#!/videoosd/videoosd.html

Copy link
Owner

@killergerbah killergerbah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this @bpwhelan . Regarding the problem with the host configuration, how likely do you think it is for an Emby user to use a non-default port? If it is not likely then I am thinking it might be reasonable to requires users to use the default port if they want to use asbplayer with Emby.

extension/src/pages/emby-page.ts Outdated Show resolved Hide resolved
extension/src/services/pages.ts Show resolved Hide resolved
extension/src/pages.json Show resolved Hide resolved
make path optional, look at both configs
Remove path from emby page json
check if ApiClient is und
Copy link
Owner

@killergerbah killergerbah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, thanks!

@killergerbah
Copy link
Owner

Ah crap, can you run prettier on the changed files? Will merge it in after that.

@bpwhelan
Copy link
Contributor Author

bpwhelan commented Feb 4, 2024

Ah crap, can you run prettier on the changed files? Will merge it in after that.

Done.

@killergerbah
Copy link
Owner

Thank you!

@killergerbah killergerbah merged commit 0cd3926 into killergerbah:main Feb 4, 2024
1 check passed
@killergerbah killergerbah added this to the Extension v1.1.0 milestone Feb 29, 2024
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