lgi-devs / lgi

Dynamic Lua binding to GObject libraries using GObject-Introspection
MIT License
440 stars 70 forks source link

python to lua rewrite, help #231

Closed vredesbyyrd closed 5 years ago

vredesbyyrd commented 5 years ago

Hi, to anyone that may not mind helping a noob, I am trying to translate a playerctl python script to lua. I am almost there.

Issue:

If the script is started while there is an open player it runs (mostly) fine. If started before there are any open players It fails to run returning a nil value.

lua:38: attempt to index a nil value (global 'player')

First, here is the python script. Its a fairly general use script to manage multiple players by name.

from gi.repository import Playerctl, GLib
manager = Playerctl.PlayerManager()

# redraw when playback status changes (play/pause/stop)
def on_playback_status(player, status, manager):
    print("status changed")

last_album = None
def on_metadata(player, metadata, manager):
    global last_album
    album_info = player.props.metadata['xesam:album']
    if player.props.player_name == ('mpv') and album_info != last_album:
        last_album = album_info
        redraw_feh()  # only call this function on album changes??

def init_player(name):
    # choose if you want to manage the player based on the name
    if name.name in ['Lollypop', 'io.github.Pithos', 'mpv']:
        player = Playerctl.Player.new_from_name(name)
        player.connect('playback-status::', on_playback_status, manager)
        player.connect('metadata', on_metadata, manager)
        manager.manage_player(player)

# redraw when player opens
def on_name_appeared(manager, name):
    print("player appeared")
    init_player(name)

# redraw when player closes
def on_player_vanished(manager, player):
    print("player vanished")

manager.connect('name-appeared', on_name_appeared)
manager.connect('player-vanished', on_player_vanished)

for name in manager.props.player_names:
    init_player(name)

main = GLib.MainLoop()
main.run()

The only real difference in functionally with the the lua re-write is I am managing any mpris compatible players, rather than by name. Also, the on_metadata function in the python version includes some code I have not implemented yet. I have only seen a couple playerctl scripts written in lua / lgi. Neither of them used a mainloop though, they were called after the player has started. I did come by a handful of programs using lgi and dbus though. Is this feasible with lgi? I assume it is and the issue is on me and my amateur hour code. If thats the case, any pointers for accomplishing this would be very appreciated.

#!/usr/bin/env lua

lgi = require('lgi')
manager = lgi.Playerctl.PlayerManager()

-- cant figure out iterating over specific players yet, 
-- so just manage all mpris players for now
function follow_player(name)
    local mpris_players = lgi.Playerctl.list_players()
    for i = 1, #mpris_players do
    player = lgi.Playerctl.Player.new_from_name(mpris_players[i])
    player.on_playback_status:connect(manager, 'playback-status')
      player.on_metadata:connect(manager, 'metadata')
      manager:manage_player(player)
    end
end

for i,v in pairs(manager.player_names) do
    follow_player(name)
end

--manager.on_name_appeared:connect('name-appeared')
--manager.on_player_vanished:connect('player-vanished')

-- redraw when player is opened
function manager:on_name_appeared(name)
    follow_player(name)
    print("player appeared")
end

-- redraw when player is closed
function manager:on_player_vanished(player)
    print("vanished")
end

-- redraw when playback status changes (play/pause/stop)
function player:on_playback_status(status, manager)
     print("status changed")
end

-- redraw when song changes
function player:on_metadata(metadata, manager)
     print("new track")
end

-- run man loop
require("lgi").GLib.MainLoop():run()
rokf commented 5 years ago

Your player variable is only created when mpris_players is not an empty table (list). This happens in the follow_player function.

Later you're trying to define two functions on the player variable on_playback_status and on_metadata.

In case that mpris_players is empty the first definition (on_playback_status) will fail because player is nil, it does not exist.

One option would be to define the two functions/methods on the player variable inside follow_player after you've created a player instance.

The way you have this written at this moment wouldn't work correctly with multiple players. The player variable gets overwritten if there are multiple players in existence. Only the last player will have the on_player_vanished and on_metadata functions defined.

psychon commented 5 years ago

I do not have Playerctl, so I cannot test this, but does the following version work?

lgi = require('lgi')
manager = lgi.Playerctl.PlayerManager()

-- cant figure out iterating over specific players yet, 
-- so just manage all mpris players for now
function follow_player(name)
    local mpris_players = lgi.Playerctl.list_players()
    for i = 1, #mpris_players do
        player = lgi.Playerctl.Player.new_from_name(mpris_players[i])
        player.on_playback_status:connect(manager, 'playback-status')
        player.on_metadata:connect(manager, 'metadata')

        -- redraw when playback status changes (play/pause/stop)
        function player:on_playback_status(status, manager)
             print("status changed")
        end

        -- redraw when song changes
        function player:on_metadata(metadata, manager)
             print("new track")
        end

        manager:manage_player(player)
    end
end

for i,v in pairs(manager.player_names) do
    follow_player(name)
end

--manager.on_name_appeared:connect('name-appeared')
--manager.on_player_vanished:connect('player-vanished')

-- redraw when player is opened
function manager:on_name_appeared(name)
    follow_player(name)
    print("player appeared")
end

-- redraw when player is closed
function manager:on_player_vanished(player)
    print("vanished")
end

-- run man loop
require("lgi").GLib.MainLoop():run()

In the Python version, on_playback_status is a normal function that is then connected to the player's playback_status signal in init_players. In your version... I doubt a lot that the :connect calls do anything useful, but I do not actually know what they are doing. The latter assignment then connects to just the last player that was seen:

Alternatively, (if you like this structure more), you can try the following version:

lgi = require('lgi')
manager = lgi.Playerctl.PlayerManager()
--
-- redraw when playback status changes (play/pause/stop)
function on_playback_status(player, status, manager)
     print("status changed")
end

-- redraw when song changes
function on_metadata(player, metadata, manager)
     print("new track")
end

-- cant figure out iterating over specific players yet, 
-- so just manage all mpris players for now
function follow_player(name)
    local mpris_players = lgi.Playerctl.list_players()
    for i = 1, #mpris_players do
        player = lgi.Playerctl.Player.new_from_name(mpris_players[i])
        player.on_playback_status = on_playback_status
        player.on_metadata = on_metadata

        manager:manage_player(player)
    end
end
[...]

Does any of this work?

vredesbyyrd commented 5 years ago

Hi, thank you both for the explanations. I figured this out late last night after reading further into PIL and lgi doc, but fell asleep before closing this which was my intention. , My solution followed the structure of your first bit of code, minus the connect calls, which as you stated are not needed here. I do prefer the structure of your 2nd bit of code. So thank you for that.

lgi = require('lgi')
manager = lgi.Playerctl.PlayerManager()

-- redraw when playback status changes (play/pause/stop)
function on_playback_status(player, status, manager)
     print("status changed")
end

-- redraw when song changes
function on_metadata(player, metadata, manager)
     print("new track")
end

-- cant figure out iterating over specific players yet, 
-- so just manage all mpris players for now
function follow_player(name)
    local mpris_players = lgi.Playerctl.list_players()
    for i = 1, #mpris_players do
        player = lgi.Playerctl.Player.new_from_name(mpris_players[i])
        player.on_playback_status = on_playback_status
        player.on_metadata = on_metadata
        manager:manage_player(player)
    end
end

-- redraw when player is opened
function manager:on_name_appeared(name)
    follow_player(name)
    collectgarbage("collect")
    print("player appeared")
end

-- redraw when player is closed
function manager:on_player_vanished(player)
    collectgarbage("collect")
    print("vanished")
end

for i,v in pairs(manager.player_names) do
    follow_player(name)
end

-- run man loop
require("lgi").GLib.MainLoop():run()

One thing of note for anyone that may try and do something similar. I had to collectgarbage("collect") whenever a player connects / disconnects, otherwise anytime a player of the same name reconnects each signal gets sent, 3x, 4x, 5x... etc. Ill close this now, and comment if I run into anything noteworthy.

Thanks again for your comments.

psychon commented 5 years ago

each signal gets sent, 3x, 4x, 5x... etc

Well...

function manager:on_name_appeared(name)
    follow_player(name)

and

function follow_player(name)
    local mpris_players = lgi.Playerctl.list_players()
    for i = 1, #mpris_players do

When a new player appears, you call follow_player which uses list_players do find all players and collect to them. So, players which already were "there" before are now being connected to another time.

I had to collectgarbage("collect") whenever a player connects / disconnects

Random guess: You need to keep the player that new_from_name gives you alive. Otherwise, the garbage collector cleans up, the proxy object is destroyed and the signals are disconnected.

vredesbyyrd commented 5 years ago

Random guess: You need to keep the player that new_from_name gives you alive. Otherwise, the garbage collector cleans up, the proxy object is destroyed and the signals are disconnected.

Okay, yeah I feel like an idiot. Just using garbagecollect() as a hammer is prob bad advice on my part. I have not worked with dbus signals much, and the convenience aspect of a library like playerctl can also be a bit sketchy if you do not familiarize yourself with what it is abstracting, like I should have done more of here.

I have to run a bunch of errands but will dig into this a bit more when I get back. First thing I want to do is make a table of playernames { 'mpv', lollypop', 'pithos' } and grab players from that rather than any mpris compatible player via lgi.Playerctl.list_players().

Thank you for the lesson!