mopidy / mopidy

Mopidy is an extensible music server written in Python
https://mopidy.com
Apache License 2.0
8.05k stars 686 forks source link

Need a way to add AND PLAY tracks #1897

Open altano opened 4 years ago

altano commented 4 years ago

I'm building a Jukebox web UI that will be consumed by multiple simultaneously-connected clients. They only have the ability to queue songs. If the tracklist is empty and the first song gets queued, I would like that song to start playing.

Right now there is no method to do that. The PlaybackController.play() will interrupt currently playing track and PlaybackController.resume() won't play the track list if nothing was playing yet.

I can obviously rig the client up to do something like:

await mopidy.tracklist.add(...);
const playbackState = await mopidy.playback.getState();
if (playbackState === "PAUSED") {
  await mopidy.playback.resume();
} else if (playbackState === "STOPPED") {
  await mopidy.playback.play();
}

The issue with this is that if two people queue up songs at the same time, they might both get back the "STOPPED" state and one person will trample over the other. I'd like to solve this by having something built-into the API that lets me both add a track and declare that it should be played if it is the only track on the tracklist and playback is stopped.

jodal commented 4 years ago

I transferred the issue to the core mopidy repo, as this would require changes here.

kingosticks commented 4 years ago

Another way to achieve to what is essentially a custom atomic collection of core operations would be to provide a Frontend implementing the command and ensure all your clients use it. This would be better if we had a mechanism for extending the websocket somehow.

On Tue, 7 Apr 2020, 08:55 Stein Magnus Jodal, notifications@github.com wrote:

I transferred the issue to the core mopidy repo, as this would require changes here.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mopidy/mopidy/issues/1897#issuecomment-610236208, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHEHKHXVX76W7636PBHZYLRLLMATANCNFSM4MC4SYFQ .

altano commented 4 years ago

Putting aside race conditions, getting this to work reliably with a single client is also effortful.

await mopidy.tracklist.add({ tracks: [track] });
await mopidy.playback.play({});

That fails if the tracklist starts empty.

My jukebox use case is simple: people queue up songs and I want something to be playing always. If the tracklist was empty and nothing was playing, the new track should start playing. If things got paused some how, they should unpause.

Does the API support this without having to do this, which feels way too complicated?:

const firstFewTracks = await mopidy.tracklist.slice({ start: 0, end: 3 });
...
const [tlTrack] = await mopidy.tracklist.add({ tracks: [track] });
if (firstFewTracks.length === 0) {
  await mopidy.playback.play({ tlid: tlTrack.tlid });
} else {
  const playbackState = await mopidy.playback.getState();
  if (playbackState === "stopped") {
    await mopidy.playback.play({});
  } else if (playbackState === "paused") {
    await mopidy.playback.resume();
  }
}

Is it the philosophy of the API to have each method be super precise in exactly what it does so that you can use them as building blocks to build behavior like my case? Or should it be more resilient to the different states and obvious intent of the caller (e.g. PlaybackController.resume() advancing the tracklist if nothing is playing)

kingosticks commented 4 years ago

That fails if the tracklist starts empty.

I must misunderstand because that works for me as I'd expect it to. What backend are you using?

When paused, calling play with no arguments should resume. Considering that, along with the above, doesn't the following do the same thing as your snippet?

await mopidy.tracklist.add({ tracks: [track] });
const playbackState = await mopidy.playback.getState();
if (playbackState != "playing") {
  await mopidy.playback.play({})
}

Keep in mind the Core API is consumed by other frontends, not just javascript webclients, and so small building blocks make sense. The intent of the caller might not always be obvious in other contexts so we don't generally bake assumptions in. To me, the idea of "resuming" when stopped makes no sense as there is nothing to resume.

kingosticks commented 4 years ago

@altano, did you have any luck with my suggestion?

altano commented 4 years ago

hey @kingosticks, I realize now that I might have thought this simpler code wasn't working because of an unrelated problem where GStreamer sporadically just fails to play after a track change with either:

ERROR 2020-04-20 03:57:04,106 [1:MainThread] mopidy.audio.gst, GStreamer error: Could not connect to server,

or

GStreamer error: Could not write to resource.

I posted about it on the Discourse forum (I didn't want to spam github issues) but my post got marked as spam and is now awaiting approval.

Once I work through that issue I will try this simpler logic again and see if it works.