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

Discord integration #363

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

william1509
Copy link

@william1509 william1509 commented Feb 3, 2023

This pull request adds discord integration to JMP. This means that discord will display which show/movie you are currently watching, as well as the time left.

image

PR is a draft because:

  • Need to fix the installer to add the discord dll. Currently the dll is manually placed with the executable. I'm not familiar with wix so if someone knows how to do that please let me know
  • Need to fix the hardcoded discord client id. As of right now, I'm using my own client id which isn't ideal. Is there an official jellyfin discord team on the developer portal?
  • Haven't tested with all the media types (e.g. music)
  • It currently only supports windows

Most of the changes are in DiscordComponent. All the other .h/.cpp are files provided by discord.

@iwalton3
Copy link
Member

iwalton3 commented Feb 3, 2023

Looks promising! I remember I added discord integration to MPV Shim but for JMP it is a lot more involved.

You might be able to use the client ID I registered for MPV Shim 743296148592263240. I also have some pre-registered icon names. https://github.com/jellyfin/jellyfin-mpv-shim/blob/master/jellyfin_mpv_shim/rich_presence.py

For this to be merged, the following enhancements would be desired:

  • Disable by default and add a checkbox in client settings. (This is important because some people don't want their friends/family to know what they are watching.)
  • Fix Linux/macOS support or disable on platforms where it is not viable.
  • Add dependencies to installer.
  • Update or have me create a new client id.

Copy link

@tecnopark076 tecnopark076 left a comment

Choose a reason for hiding this comment

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

hy

@Radiicall
Copy link

Radiicall commented Feb 7, 2023

I have a rust project that does this but it uses the jellyfin api, i dont know if it'll be any help but https://github.com/Radiicall/jellyfin-rpc It supports Windows, Linux and macOS but its written in an entirely different language from this entire project.

@xAphex
Copy link

xAphex commented Jun 9, 2024

any updates with this?

@Retro8bit
Copy link

This is a great idea.

Although personally, I would much rather be required to create my own Discord application via the developer portal instead and provide my own application ID for privacy reasons. Since images uploaded by the application to Discord to be used for rich presence are also viewable by the application's developer in the Discord developer portal.

There is also a 300 image limit for Rich Presence art assets per application, so every Jellyfin client using the same application ID to communicate with Discord is not a good idea.
Discord documentation: https://discord.com/developers/docs/rich-presence/overview#assets

There's a good plugin for the music player, MusicBee, that I think implements Discord Rich Presence very well.
Link to the plugin: https://www.getmusicbee.com/addons/plugins/156/discordbee/
Link to the plugin's GitHub: https://github.com/sll552/DiscordBee

@Radiicall
Copy link

This is a great idea.

Although personally, I would much rather be required to create my own Discord application via the developer portal instead and provide my own application ID for privacy reasons. Since images uploaded by the application to Discord to be used for rich presence are also viewable by the application's developer in the Discord developer portal.

There is also a 300 image limit for Rich Presence art assets per application, so every Jellyfin client using the same application ID to communicate with Discord is not a good idea.
Discord documentation: https://discord.com/developers/docs/rich-presence/overview#assets

There's a good plugin for the music player, MusicBee, that I think implements Discord Rich Presence very well.
Link to the plugin: https://www.getmusicbee.com/addons/plugins/156/discordbee/
Link to the plugin's GitHub: https://github.com/sll552/DiscordBee

The problem with art that you are referring to is a non-issue, having the asset as a url will not add it in there and will not make it visible to the application dev.
Application IDs are public (and can be used in any app without auth) so this would be awful for any games or applications that support proper rich presence seeing as according to you any image that gets used with the application ID automatically gets added there..

I even double checked this just now as I have an app in Discord's dev portal that uploads images to imgur and uses jellyfin images directly, the tab is completely empty 0/300 assets.

If you're looking for privacy then having your playback as a visible status on Discord is not something you want

@Retro8bit
Copy link

The problem with art that you are referring to is a non-issue, having the asset as a url will not add it in there and will not make it visible to the application dev. Application IDs are public (and can be used in any app without auth) so this would be awful for any games or applications that support proper rich presence seeing as according to you any image that gets used with the application ID automatically gets added there..

I even double checked this just now as I have an app in Discord's dev portal that uploads images to imgur and uses jellyfin images directly, the tab is completely empty 0/300 assets.

If you're looking for privacy then having your playback as a visible status on Discord is not something you want

This is good to know, thanks for taking the time to explain. I completely missed the part about using external URLs in the documentation I linked somehow.
Regardless, would love to see this added to JMP.

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.

6 participants