Gnome-shell-extension-gsconnect: High gjs CPU usage when playing music in Audacious

[ferdnyc]

Also i have this kind of problem when listening to music with Audacious.
I'm on Ubuntu 18.10
There is a screenshot of htop when listening to music:

And there is what "journalctl /usr/bin/gjs -f" returns:
dic 07 17:13:23 marco-Swift-SF314-51 gjs[1499]: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
dic 07 17:13:23 marco-Swift-SF314-51 gjs[1499]: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
dic 07 17:13:23 marco-Swift-SF314-51 gjs[1499]: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
dic 07 17:13:23 marco-Swift-SF314-51 gjs[1499]: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
dic 07 17:13:23 marco-Swift-SF314-51 gjs[1499]: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
dic 07 17:13:23 marco-Swift-SF314-51 gjs[1499]: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
dic 07 17:13:23 marco-Swift-SF314-51 gjs[1499]: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
dic 07 17:13:23 marco-Swift-SF314-51 gjs[1499]: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
dic 07 17:13:23 marco-Swift-SF314-51 gjs[1499]: g_object_unref: assertion 'G_IS_OBJECT (object)' failed

_Originally posted by @Dolland in https://github.com/andyholmes/gnome-shell-extension-gsconnect/issues/330#issuecomment-445282433_

[ferdnyc]

Also i have this kind of problem when listening to music with Audacious.

I'm not so familiar with audacious, but it seems like the MPRIS plugin might be emitting position changes very frequently (https://github.com/audacious-media-player/audacious-plugins/blob/master/src/mpris2/plugin.cc#L170-L183):

static void update (void * object)
{
    int64_t pos = 0;
    int vol = 0;

    if (aud_drct_get_playing () && aud_drct_get_ready ())
        pos = (int64_t) aud_drct_get_time () * 1000;

    vol = aud_drct_get_volume_main ();

    g_signal_handlers_block_by_func (object, (void *) volume_changed, nullptr);
    g_object_set ((GObject *) object, "position", pos, "volume", (double) vol / 100, nullptr);
    g_signal_handlers_unblock_by_func (object, (void *) volume_changed, nullptr);
}

And (https://github.com/audacious-media-player/audacious-plugins/blob/master/src/mpris2/plugin.cc#L360):

timer_add (TimerRate::Hz4, update, object_player);

Compare this with the MPRIS Specification which says:

The org.freedesktop.DBus.Properties.PropertiesChanged signal is not emitted when this property changes.

Probably you should file an upstream report with audacious for this, and ask if these signals are being emitted over DBus.

_Originally posted by @andyholmes in https://github.com/andyholmes/gnome-shell-extension-gsconnect/issues/330#issuecomment-445420052_

[ferdnyc]

Audacious does indeed output PropertiesChanged updates on Position at the rate of approx. four per second when playing, on my system. Compared to mpdris2 (the MPRIS2 monitor for the MPD Music Player Daemon, and VLC, which only output messages for events like start, stop, seek, and song-change.

[ferdnyc]

Ah, yeah that's way to often, especially since it's actually microsecond measurement. The high cpu with GSConnect is because it's relaying each change to connected devices.

Generally only the Seeked() signal should be emitted and MPRIS clients should run their own "local" timer starting from Position, otherwise this kind of performance issue will happen for all clients.

_Originally posted by @andyholmes in https://github.com/andyholmes/gnome-shell-extension-gsconnect/issues/330#issuecomment-445425725_

[ferdnyc]

Submitted upstream as Audacious issue 849.

[Dolland]

Thanks for the support!
So, if I want to mitigate the problem, one solution could be to deactivate MPRIS plugin, right?
Thanks also for your report on audacious channels.
There is something else I can do to help solving this issue?

[ferdnyc]

@Dolland

Thanks for the support!
So, if I want to mitigate the problem, one solution could be to deactivate MPRIS plugin, right?

If you were to deactivate Audacious' MPRIS2 plugin, that should solve this, yes. You'll still be able to control other MPRIS2-compatible media players from your Android device(s).

Or you could deactivate GSConnect's proxying, if you prefer to keep MPRIS2 enabled in Audacious. @andyholmes can probably comment on the best way to do that, it'll be one of:

1. Switch Sharing > Media Players to "Off" for each paired device in GSConnect Settings
2. Uncheck Advanced > Plugins > MPRIS for each paired device in GSConnect Settings

(Actually, the second option will _remove_ the switch for the first one completely. But it's probably sufficient to just use the "Media Players" switch in Sharing, aka the first option.)

[Dolland]

Ok, i'll try ASAP and I'll report here if i solve!

EDIT:
Confirmed: Disabled MPRIS Server on Audacious, now gjs keeps ~2% CPU when listening to music!

[andyholmes]

Excellent another not-our-bug bug :D

@ferdnyc thanks for filing the upstream bug. There's only so much triaging I can bear to do on my tablet keyboard :P

[ferdnyc]

Excellent another not-our-bug bug :D

Unfortunately, appears it's not quite _their_ bug, either.

It is currently not possible to disable these signals due to a missing feature in GDBus, see:
https://gitlab.gnome.org/GNOME/glib/issues/542

This has been known for over 6 years; if you would like to see it fixed, please talk to the GNOME project.

The missing feature, I gather from the linked report, is actually in gdbus-codegen, which they're using to generate their DBus interface code from the DBus interface XML, and which still — _still_ — ignores the EmitsChangedSignal = false annotation for Properties... despite the fact that this was first reported, specifically in the MPRIS2 context, over six and a half years ago. A patch was submitted, received a little initial review, and the author submitted an updated version around 5.5 years ago... but never received any further response.

Which is an almost too-perfect illustration of the kind of things we've been talking about in #368.

[amivaleo]

Which is an almost too-perfect illustration of the kind of things we've been talking about in #368.

Really?
I mean: I can't understand mostly anything of what you're talking about here. ^_^
So... Well... 🤷‍♂️

[andyholmes]

A patch was submitted, received a little initial review, and the author submitted an updated version around 5.5 years ago... but never received any further response.

Well, I did a half-assed rebase of that patch and opened MR !532. The pipeline is breaking for some reason, but I think it's actually passing the test suite. :confused:

I hate to be an apologist, because 6 years is a long time, but man reviewing PR/MR's is so much nicer than the old "download my flaky patch and eyeball the code". I pity the fool still using bugzilla nowadays.

[ferdnyc]

Really?
I mean: I can't understand mostly anything of what you're talking about here. ^_^

Oh, I just meant regarding the Gnome developer support issues.

You can't:

1. Impose all of these design restrictions
2. Deprecate tons of existing features people rely on (without providing any sort of replacement)
3. File bugs telling app developers they need to _remove_ features from their software, simply because your desktop environment has decided to do away with the functionality, so now they "have to decide if you are a GNOME app, an Ubuntu app, or an XFCE app" (Meaning: conform to one faction's standards and to hell with the other groups.)

Eight years later and still that one leaves me, in the words of the Transmission developer at the time, "`*speechless*`".

1. Basically act like the work you're doing is a gift to an ungrateful community who can't appreciate your Grand Vision™

...on one hand, but then on the other hand, when an issue is reported by _multiple_ developers due to missing support in the toolchains you provided them, to implement the functionality you specified ("you" being used collectively here, to mean "the Gnome developers"), you can't leave that broken for six YEARS.

Not when it's actively causing bugs for developers who actually _have_ made the decision that they "are a GNOME app" (or, they're writing one). I mean, this is all Gnome-internal stuff, it's not even like there are outside dependencies at play. DBus, MPRIS2, all of this stuff is _theirs_. Yet one part of it has been breaking another since Mitt Romney ran for President.

[ferdnyc]

Well, I did a half-assed rebase of that patch and opened MR !532. The pipeline is breaking for some reason, but I think it's actually passing the test suite.

I hate to be an apologist, because 6 years is a _long_ time, but man reviewing PR/MR's is so much nicer than the old "download my flaky patch and eyeball the code". I pity the fool still using bugzilla nowadays.

Yeah, there's something up with the generated code. When it bails with:

gio/xdp-dbus.c: In function ‘gxdp_documents_skeleton_set_property’:
gio/xdp-dbus.c:3284:62: error: ‘GDBusPropertyInfo’ {aka ‘const struct _GDBusPropertyInfo’} has no member named ‘emits_changed_signal’
           _gxdp_documents_property_info_pointers[prop_id - 1]->emits_changed_signal)
                                                              ^~
gio/xdp-dbus.c: In function ‘gxdp_open_uri_skeleton_set_property’:
gio/xdp-dbus.c:4797:61: error: ‘GDBusPropertyInfo’ {aka ‘const struct _GDBusPropertyInfo’} has no member named ‘emits_changed_signal’
           _gxdp_open_uri_property_info_pointers[prop_id - 1]->emits_changed_signal)
                                                             ^~

"‘GDBusPropertyInfo’ {aka ‘const struct _GDBusPropertyInfo’} has no member named ‘emits_changed_signal’" is _true_, because emits_changed_signal is a member of the _ExtendedGDBusPropertyInfo struct. So, something's accessing data at the wrong level.

[andyholmes]

Hmm, well I'll have to come back to that later. Hopefully someone else will jump in who knows more about it, although to be honest I'm not sure why audacious hasn't just edited the C generated by gdbus-codegen in 6 years either. :confused:

Closing this as up-up-and-astream!

[ferdnyc]

although to be honest I'm not sure why audacious hasn't just edited the C generated by gdbus-codegen in 6 years either.

I confess that question had occurred to me as well, but... ¯\_(ツ)_/¯

[ferdnyc]

Ooh, hey, fixed! Reportedly, anyway.

Apparently Lindgren was able to get the gdbus-codegen fix working from Schnouki's/your patch, and with their XML properly annotated says that the Position update signals are no longer emitted. Not sure when that'll hit Audacious releases, but at least we won't make it to _seven_ years for this bug's lifetime... probably.

ETA: (Aha, I see now that it was Schnouki who sorted out the code — makes sense considering he wrote it.)