andyholmes / valent

Connect, control and sync devices
https://valent.andyholmes.ca
Other
561 stars 18 forks source link

Flatpak can't see MPRIS album art from Chrome/Rhythmbox/etc #652

Open retrixe opened 4 months ago

retrixe commented 4 months ago

Current Behavior

The phone should see the MPRIS album art and there should be no errors logged.

Expected Behavior

The phone has no thumbnail and Valent logs an error saying it's unable to find the album art.

Allowing filesystem=host and /tmp does fix the issue, but is undesirable. Maybe flatpak-spawn could be used to work around the issue?

Desktop

GNOME Shell

Other Desktop

No response

Operating System

Fedora 40 + GNOME 46

Installed from

Nightly Flatpak

Version

1.0.0.alpha

Devices

No response

Plugins

MPRIS

Logs

13:52:53.2469            valent-mpris-plugin:    DEBUG: Failed to upload album art: Error when getting information for file “/tmp/.com.google.Chrome.J2nRNM”: No such file or directory
13:53:15.0162            valent-mpris-plugin:    DEBUG: Failed to upload album art: Error when getting information for file “/home/ideapad/.cache/rhythmbox/album-art/006”: No such file or directory

Screenshots

No response

andyholmes commented 4 months ago

I'm labeling this a bug, even though I'm not sure it's our bug. Generally I would categorize this an MPRIS bug, because URIs as the resource type for albumart is frankly bonkers and there's no reason not to just include the bytes in the Metadata mapping.

In fact, this gets worse for some sandboxed browsers, because they can't actually report the true host path to the albumart (assuming it's actually on the local filesystem). In principle, I think a data:image/jpeg;charset=utf-8;base64, URI could be used by implementations, but that's not exactly more reasonable or supported.

I'm not sure flatpak-spawn is a viable workaround, assuming you mean something like flatpak-spawn --host cp X Y. Ideally we could drop the permissions for flatpak-spawn, because AFAIK there's nothing stopping an application from rewriting it's own permission overrides. However, flatpak-spawn is really the only thing that makes the runcommand plugin useful, and we need --socket=session-bus for notifications so that permission is kind of moot anyways.

Sandboxing is hard when your application does everything you'd want a trojan to do :thinking:

retrixe commented 4 months ago

I was thinking more along the lines of flatpak-spawn --host cat /path/to/album_art.png (assuming valent is in a flatpak) and using the output, it's a lot better than cp although less than ideal (should be workable though?)

Ideally, yeah MPRIS should avoid URLs lol, but proposing any change to the spec and having it be implemented by apps seems a long way off

andyholmes commented 4 months ago

To be honest, I'm really not a fan of trying to fixing out-of-repo bugs, especially with MPRIS.

I'll think about it, but a solution that didn't involve hacking around user configurable permissions would be a lot more convincing.

rohmishra commented 1 month ago

@retrixe MPRIS is really due for an update. It should handle remote sinks a lot better too (remote media like Cast).

andyholmes commented 1 month ago

How do you mean? Does that relate to Flatpak?

Sorry, I misread your comment there :P