Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Pass SVG images through Slim::Web::ImageProxy without resizing #1181

Open
wants to merge 1 commit into
base: public/9.0
Choose a base branch
from

Conversation

mavit
Copy link
Contributor

@mavit mavit commented Oct 15, 2024

This seems to work fine with the default and Material skins.

It doesn’t work with the Squeezer Android app, causing the album artwork to be displayed as a dark grey rectangle when SVG is attempted. I think I can live with that, given that it’s not much worse than the placeholder “no artwork” image we’d get otherwise.

I no-longer have a Squeezebox Controller to test against.

Motivation

I would like this for my PipeWire plugin, which asks GTK for the icon of an application that’s streaming audio to the plugin, and often gets given an SVG.

@mavit mavit force-pushed the imageproxy-svg-passthrough branch from 9a1545c to 9b337f1 Compare October 15, 2024 18:19
Copy link
Member

@michaelherger michaelherger left a comment

Choose a reason for hiding this comment

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

Would you know why the code on line 145 would not cover your use case? I already tried to make SVG be pulled directly.

@mavit
Copy link
Contributor Author

mavit commented Oct 17, 2024

An example usage (taken from the Material skin) would be:

<div class="np-bar-image"><img src="/imageproxy/pipewire/icon-name/org.gnome.Totem.svg/icon_1024x1024_f.png" loading="lazy" onerror="this.src=DEFAULT_COVER" class="np-cover"></div>

Although the skin is asking for a PNG, if we give it an SVG instead, that's fine, the browser knows what to do with it.

@michaelherger
Copy link
Member

michaelherger commented Oct 17, 2024

What would the "raw" URL of that image file be? Is a "pipewire" plugin or something involved? The image proxy usually is only used for external images, not local files. And there's no "http" in that URL.

@mavit
Copy link
Contributor Author

mavit commented Oct 17, 2024

PipeWire gives us either the name of a system icon which we can get, say, GTK to look up for us (and in this example would resolve to /usr/share/icons/hicolor/scalable/apps/org.gnome.Totem.svg) or a blob containing PNG data (according to the spec, at least, although I haven’t seen the latter done in practice).

I asked about this at
https://forums.slimdevices.com/forum/developer-forums/developers/1721619-provding-cover-images-from-a-plugin, and image proxy was the suggestion.

PipeWire, if you’ve not heard of it, is an audio (and video) multiplexer. It replaces (and borrows a bunch of code from) PulseAudio, the previously-dominant audio daemon on Linux. We can capture audio that other applications stream to it (which is the point of my plugin), and also get metadata about those streams/applications to use to populate LMS's now-playing.

@michaelherger
Copy link
Member

michaelherger commented Oct 17, 2024

So you're using your own Plugin to create that URL? I assume you've implemented your own image proxy hook, handling the pipewire part? I think you should handle things in there, as IMHO the image proxy already does handle SVG files correctly by not trying to resize them, but returning them directly. Would you have a pointer to your plugin?

@michaelherger
Copy link
Member

I'd recommend you implement a page handler to serve those images in your plugin. We can't implement this in core LMS as it will not work with many clients. All Jivelite/Squeezeplay based clients would fail. If a client is requesting a specific format, we must respect this - or fail gracefully. But sending back a different file format might be a poorer solution for most of those clients.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants