Conversation
|
Just had a little play with this. Nice job! A few things I noticed:
Now, it also seems to index media from streaming providers. When opening the 'Artist' folder, it seems to call I think the goal here is to only serve the content from the local/smb file providers? If so, can we configure the plugin to only advertise media items that are provided by local/smb? |
Co-authored-by: Marcel van der Veldt <m.vanderveldt@outlook.com>
… into dlna-server
|
@MarvinSchenkel I think I have to do something. What do you think of only scanning at the track level so you would see the artist then album but inside would be empty if there were no local tracks? Otherwise I think it could be very slow if the provider had to scan every track of an artist just in case one had a local mapping? |
The library functions should have a Add a constant on the top: Then inside Of course we will also need this for albums, tracks, album_tracks... |
|
@MarvinSchenkel Yes I was off on a tangent for a second as I'm doing a million things here. I havent had a chance to test it but I pushed a fix (hopefully!) |
|
This PR has major architectural challenges for which I think we currently do not have the capacity at all. We have about 80 open user issues reports which all have a higher priority than adding this niche feature to our codebase. Marked as draft and we will revisit this when capacity is in a better place. |
🔒 Dependency Security Report✅ No dependency changes detected in this PR. |
| """Handle device description request.""" | ||
| base_url = self.mass.streams.base_url | ||
|
|
||
| device_xml = f"""<?xml version="1.0"?> |
There was a problem hiding this comment.
should this come from a helper ?
| charset="utf-8", | ||
| ) | ||
|
|
||
| async def _handle_track_stream(self, request: web.Request) -> web.StreamResponse: |
There was a problem hiding this comment.
I don't like this because it opens the door for piracy.
we need at least a concept of sessions and attaching these requests to a player(queue) and limit concurrent streams / detect rapid downloads etc.
Also its duplicating logic that we already have in the streams controller.
| } | ||
|
|
||
|
|
||
| class SSDPServer: |
There was a problem hiding this comment.
look what you can unify from the new discovery controller instead of reinventing the wheel
|
DLNA is an outdated, very insecure protocol. I'm not so sure if we really want to add this support to our codebase. |
|
I agree with you and the user experience varies considerably by the client software. However, this was actually your feature request 😀 Currently, it is at number 14 of the unimplemented requests with 26 votes https://github.com/orgs/music-assistant/discussions?discussions_q=is%3Aopen+sort%3Atop+-label%3Aimplemented |
Yeah, I don't think 26 votes satisfies introducing a security risk. Let's focus on high demand requests. This PR can be accepted if we find a way to fix the security risks and the open door for piracy - otherwise I'm afraid it has been a waste of your time, sorry! |
My first attempt at a plugin!
This creates a DLNA server which allows playback of local file system files.
In my testing I tried:
Android BubbleUPNP: Worked very well allowing queueing of all artist tracks, or all album tracks or individual tracks
VLC on WIndows: Allowed playback of individual tracks
nPlayer Lite on iOS: Played back individual tracks and albums.
Overall, the plugin seemed to be working and variations in functionality I assess are due to the respective clients. BubbleUPNP for example provides a rich experience with full artwork and full capabilities.
Streaming content is also served
Hopefully satisfies https://github.com/orgs/music-assistant/discussions/388