Skip to content

Conversation

@chicco-carone
Copy link
Contributor

This pull request introduces enhancements to the Plex integration in Music Assistant, focusing on improving stream data handling and adding a new plugin provider for reporting playback timeline events. The most important changes include restructuring stream data, updating playback tracking logic, and introducing the PlexConnect plugin provider.

Enhancements to Plex Stream Data Handling:

  • Refactored the get_stream_details method to use a structured dictionary (plex_data) for stream metadata, replacing the direct use of plex_track in the data field of StreamDetails. This improves clarity and maintainability and is used by the plex connect plugin to report back to the server the data.
  • Updated the on_streamed method to use the new plex_data structure for marking tracks as played, replacing direct attribute access with dictionary keys.

Introduction of the PlexConnect Plugin Provider:

  • Added a new PlexConnect plugin provider to monitor and report Plex playback timeline events. This plugin adds support to synchronize track playback back to the plex server.
  • Created a corresponding manifest.json file for the PlexConnect plugin, specifying its metadata, requirements, and documentation link.

This pr contains the remain of the code from #2228

Copilot AI review requested due to automatic review settings July 10, 2025 18:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new PlexConnect plugin to report playback timeline events back to Plex and refactors how Plex stream metadata is handled.

  • Refactored get_stream_details and on_streamed to package Plex metadata into a structured plex_data dict
  • Introduced PlexTimelineReporter to send playback, pause, and stop events to Plex
  • Added plugin manifest and wiring to load/unload the timeline reporter

Reviewed Changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 3 comments.

File Description
music_assistant/providers/plex_connect/timeline.py New PlexTimelineReporter class for sending timeline updates
music_assistant/providers/plex_connect/manifest.json Plugin manifest for PlexConnect
music_assistant/providers/plex_connect/init.py Plugin provider setup/unload logic
music_assistant/providers/plex/init.py Updated Plex provider to emit structured plex_data
Comments suppressed due to low confidence (1)

music_assistant/providers/plex_connect/timeline.py:1

  • Consider adding unit or integration tests for PlexTimelineReporter to cover the various state transitions and ensure timeline updates are sent correctly.
"""

@OzGav
Copy link
Contributor

OzGav commented Jul 11, 2025

I think this should be split into smaller PRs again. The plugin should be its own?

if not player.available or player.powered is False:
self.logger.info("Player %s is no longer available or powered off", pid)
pause_pos = int(data["elapsed_time"] * 1000)
self.logger.info("Arresto riproduzione per spegnimento player...")
Copy link
Contributor

Choose a reason for hiding this comment

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

Italian?

@chicco-carone
Copy link
Contributor Author

I think this should be split into smaller PRs again. The plugin should be its own?

I don't think since it's just the plugin and the other part in the Plex integration is required to make it work and generally it's a better implementation than before

media_part: PlexMediaPart = media.parts[0]
audio_stream: PlexAudioStream = media_part.audioStreams()[0]

plex_data = {
Copy link
Member

Choose a reason for hiding this comment

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

One possible small enhancement is to specify the dataformat with a TypedDict so you have a bit of help in the editor what to expect for the fields.

@marcelveldt
Copy link
Member

Please fix the last remaining mypy typing issues and then this PR is good to go

@chicco-carone
Copy link
Contributor Author

I ran mypy and all the issues are not from the plex files so I'm not sure what to fix

Comment on lines +46 to +51
# Config Entries are used to configure the Provider if needed.
# See the models of ConfigEntry and ConfigValueType for more information what is supported.
# The ConfigEntry is a dataclass that represents a single configuration entry.
# The ConfigValueType is an Enum that represents the type of value that
# can be stored in a ConfigEntry.
# If your provider does not need any configuration, you can return an empty tuple.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Config Entries are used to configure the Provider if needed.
# See the models of ConfigEntry and ConfigValueType for more information what is supported.
# The ConfigEntry is a dataclass that represents a single configuration entry.
# The ConfigValueType is an Enum that represents the type of value that
# can be stored in a ConfigEntry.
# If your provider does not need any configuration, you can return an empty tuple.

Plex content playback.
"""
self.logger.debug("Setting up Plex timeline reporter...")
self._session = aiohttp.ClientSession()
Copy link
Member

Choose a reason for hiding this comment

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

use the existing global clientsession instead of creating your own:

mass.http_session or mass.http_session_no_ssl

Comment on lines +80 to +82
if self._session:
await self._session.close()
self._session = None
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if self._session:
await self._session.close()
self._session = None

not needed when global clientsession is used

Comment on lines +124 to +130
def _validate_plex_data(self, plex_data: dict) -> bool:
required_keys = ["rating_key", "server_url", "token", "machine_identifier"]
if not all(k in plex_data for k in required_keys):
missing = ", ".join(k for k in required_keys if k not in plex_data)
self.logger.debug("Missing required Plex data in streamdetails: %s", missing)
return False
return True
Copy link
Member

Choose a reason for hiding this comment

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

So this seems to hard depend on the plex provider itself to send this data ?
Maybe then it makes sense to add "depends_on" in the manifest and set it to plex

Copy link
Contributor Author

@chicco-carone chicco-carone Aug 16, 2025

Choose a reason for hiding this comment

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

Yes without the Plex integration it doesn't work. Does the dependency need to be added in the manifest.json for the provider?

Copy link
Member

Choose a reason for hiding this comment

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

manifest

@marcelveldt marcelveldt marked this pull request as draft September 10, 2025 18:36
@marcelveldt
Copy link
Member

Marked as draft, awaiting the review comments to be addressed.
Once these are resolved, please mark the PR ready for review again, thanks!

@arsaboo
Copy link

arsaboo commented Oct 6, 2025

@chicco-carone Looking forward to this one.

This may be outside the scope of this PR, but can we also sync ratings between Plex and MA?

@marcelveldt
Copy link
Member

Superseded by #2512

Sorry for having to abandon your work here @chicco-carone but the state was stale for a while and a simpler approach came in which is now already merged. Feel free to improve on top of that later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants