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

Allow Chromecast provider to handle/avoid images which are "too big" #1806

Merged
merged 2 commits into from
Dec 23, 2024

Conversation

ijc
Copy link
Contributor

@ijc ijc commented Dec 16, 2024

My Chromecast Ultra crashes when presented with cover images on the order of 5000x5000 pixels (I've not determined the actual limit, 2048x2048 is ok though).

Allow users to avoid this problem by providing an advanced option per player which downscales the image to a specified size.

Fixes: music-assistant/hass-music-assistant#3285

@marcelveldt
Copy link
Member

I don't think this should even be an option. Let's just limit all images that are sent to a player directly to 512x512, which is also the max for some other players like airplay

@ijc
Copy link
Contributor Author

ijc commented Dec 19, 2024

My logic was that this was in the same set of things as the "force mp3" behaviour, but that's fine, I can make the change. Do you want this to happen in the core PlayerQueuesController level or per-player provider?

@marcelveldt
Copy link
Member

marcelveldt commented Dec 19, 2024

My logic was that this was in the same set of things as the "force mp3" behaviour, but that's fine, I can make the change. Do you want this to happen in the core PlayerQueuesController level or per-player provider?

Yeah, but enabling the mp3 mode is more a choice where this is more a bug; sending a large image will simply crash the device.

Let's just adjust the "get_image_url" with a "size=512" parameter in the individual player providers, starting with cast.

Scherm­afbeelding 2024-12-19 om 17 29 34

@ijc
Copy link
Contributor Author

ijc commented Dec 20, 2024

Ah, I misunderstood the mp3 option as being a workaround for buggy players too.

... adjust the "get_image_url" with a "size=512" parameter in the individual player providers,...

Changing just that one in update_flow_metadata is not sufficient IME.

The one in PlayerQueuesController.player_media_from_queue_item (here) also needs to change, the URL from that one ends up in the per player play_media and enqueue_next_media hooks (as part of the PlayerMedia parameter) which is why I was patching in _handle_limit_image_size into those locations.

In this PR I didn't touch the one in update_flow_metadata at all and things work for me when playing back from the local provider. Looks like update_flow_metadata is only used for queue.flow_mode (which isn't used in chromecast player?) or for radio streams (which I've never tried).

@ijc
Copy link
Contributor Author

ijc commented Dec 20, 2024

I added some radio stations and those do go via update_flow_metadata as expected -- none of the ones I tried actually had an image url, but if they did I'd expect them to be on the small side and not trigger the issue anyway.

@ijc ijc force-pushed the chromecast-images-too-big branch from d0b7a6f to 36d58e1 Compare December 20, 2024 10:22
@marcelveldt
Copy link
Member

Changing just that one in update_flow_metadata is not sufficient IME.

That was just a code example. It would need to be adjusted in all parts that handle the image from the queue item.
If you want to be 100% certain that a resized image is sent, we route everything through the image proxy.

Idea can be that we go with your patch now and if this pops up again, we know we have to adjust it to the proxy everywhere ?

ijc added 2 commits December 21, 2024 10:19
Debian systems do not have Python3 as `python`. With this change developers on
that platform can use:

```console
$ PYTHON=python3 ./scripts/setup.sh
```
My Chromecast Ultra crashes when presented with cover images on the order of
5000x5000 pixels (I've not determined the actual limit, 2048x2048 is ok
though).

This sets the default size in the player queue to 512x512 which affects all
players, as well as a chromecast specific codepath. Of the other callers of
`get_image_url` airplay already sets a size of 500x500 while the slimproto and
sonos players do not set any limit.

Fixes: music-assistant/hass-music-assistant#3285
Copy link
Member

@marcelveldt marcelveldt left a comment

Choose a reason for hiding this comment

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

Thanks!

@marcelveldt marcelveldt merged commit 10adb4e into music-assistant:dev Dec 23, 2024
3 checks passed
@ijc ijc deleted the chromecast-images-too-big branch December 23, 2024 10:11
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.

Chromecast Player Crashes / Chomecast Reboots
3 participants