search

StevenLooman / async_upnp_client

Async UPnP Client for Python
Other
46 stars 37 forks source link

Problem with media URI containing query parameters #121

Closed koying closed 2 years ago

koying commented 2 years ago

Hi, I'm using this library through Home Assistant and encounter an issue in async_set_transport_uri when the uri contains query parameters.

From, e.g., https://my.jelly.server/Audio/c0b4564b541312845ccf9115bb22e919/stream?static=true&MediaSourceId=c0b4564b541312845ccf9115bb22e919&api_key=e11dc415191147158c4a6bed7b4151d7, the resulting uri is https://my.jelly.server/Audio/c0b4564b541312845ccf9115bb22e919/stream?static%3Dtrue%26MediaSourceId%3Dc0b4564b541312845ccf9115bb22e919%26api_key%3De11dc415191147158c4a6bed7b4151d7 when it comes to mopidy via upmpdcli, and mopidy fails to render it. The uri doesn't work in BubbleUpnp either.

If I remove the quote_plus from https://github.com/StevenLooman/async_upnp_client/blob/0.24.0/async_upnp_client/profiles/dlna.py#L802 , both mopidy and bubleupnp render the file.

I'm not too sure if it's an issue of this library or of the renderers, really, but the fact that both of the above show the same behavior is intriguing, to say the least.

StevenLooman commented 2 years ago

Hi @koying, thank you for this issue. To be fair, it has been a while since I've looked at DLNA/DMR. @chishm has done a lot more recently however. @chishm do you have the time to look at this?

chishm commented 2 years ago

I'm not actually sure that async_upnp_client should be modifying the URI like that. The code pre-dates my efforts, but I'm happy to make the fixes. In this case I think it should just be passing the media_url through directly.

koying commented 2 years ago

I think it should just be passing the media_url through directly

That what my tests seem to indicate, indeed, but not sure either why it was there in the first place ;) Maybe some other renderer requiring it?

StevenLooman commented 2 years ago

I'm unsure. Perhaps there were some escaping problems before, later on fixed the proper way and this was a left over.

chishm commented 2 years ago

OK, I'll make the obvious fix of passing the URL through directly and test it out on my end before putting in a PR.

StevenLooman commented 2 years ago

Thank you @chishm

StevenLooman commented 2 years ago

I'll create a new release.

StevenLooman commented 2 years ago

I've released 0.26.0