First Impressions of 2.0.0-rc1

[Kerosel]

(Sorry this is so long. I am a very verbose person. It's just the way I am)

i stepped up to 2.0.0-rc1 to try to flush out a problem in 1.4.x. The problem is solved in 2.0.0-rc1 but I found a few other things that struck me as odd while getting to know the new version. Rather than spam a bunch of Issues that might not actually be issues, I thought I'd open it up for discussion.

Default Playlist IDs

In 1.3.x and 1.4.x, if you didn't explicitly specify an id for a playlist, it would get one based on the directory or file name. playlist("/mnt/media/<...>/comedy") would have the id "comedy". In 2.0.0.-rc1 it appears if you don't assign an id to a playlist it gets a generic name of "playlist_#" with the number increasing with each playlist. This is a bit confusing in the logs and when trying to send commands to the playlists via the telnet interface.

This isn't anything that's broken, just cosmetic convenience. Of course I can add id arguments to each playlist statement and maybe that's the correct answer, But I liked the convenience of the automatic naming, since my directories and playlist files are usually named for what they are ("comedy", "morning", "evening", "longform", etc).

(I used to put the .m3u extension on my list files but that named the playlist "morning(dot)m3u" which was super-cumbersome to type so I got rid of the extensions)

Playlist "next" function in telnet interface

I went to issue a playlist.next command in the telnet interface and was surprised to get "OK" in return. And more surprised when my playlist advanced to the next item. The log said that "next" is deprecated and I should use "skip" instead, which is fair. Except that in 1.4.x (and I think 1.3.x) "next" didn't move the playlist forward, it listed the next 12 items in the playlist queue with indications of which items were queued or currently playing.

I like this function, especially on my playlists that only play at certain times. I can peek at what's coming up before it is put in the stream (and skip it if I'm not feeling it). I admit that calling it "next" is confusing and seems like it should move the playlist to the next item. Could the queue printout function be returned and maybe named something more appropriate? "upnext"? "queue"?

Fallback (I think?) plays an extra item before switching when source becomes available

This was present in 1.4.x as well but I'm pretty sure not in 1.3.x. It seems to be in the fallback function, but maybe it's somewhere else and manifests itself at the fallback statement.

I have a fallback statement:

fallback([upnext,album,special,music])

where "upnext" is a request.queue, "album" is a playlist, "special" is a switch statement and "music" is another switch.

The idea is that pushing something into the "upnext" queue will cause it to be the next thing played. However, once there is an item in the "upnext" queue, the current item finishes and another is played from that source before fallback goes to the "upnext" source. Doing a "request.trace" on the RID of the upnext queue item shows it isn't even resolved until the "extra" item starts playing.

The same behavior is observed with the "album" playlist: it starts with an empty file and is watched. Once I copy a list into the file, the current item and one more will play before fallback kicks over to "album". I haven't paid close enough attention but I would bet the same thing happens when one of the "special" events becomes available.

I can live with this one... I just know that "upnext" or "album" won't start until after the next song. But it seems like where timing is a factor (say, a top-of-the-hour ID on a radio station) the ID might be switched in to play at the first available spot after the top of the hour, but if it doesn't play until after another song the ID could be delayed for a bit.

Bonus: Why does order of Switch items matter?

This isn't a bug or a problem (as least as far as I know?), but I ran into it when converting my script to 2.0.0-rc1 and it confused me. My "special events" switch statement was the following in 1.3.x and 1.4.x:

special = switch (id="specswitch", [
      ({1w and 05h-00h}, tracker),                  # MOD MONDAYS!
      ({3w and 17h-19h}, comedy),                   # Do the Comedy Mix on Wednesdays
      ({5w and 18h-19h}, delay(7200.0, longform))   # Play a Longform Mix on Friday (don't re-fire within 2 hours)
])

where "tracker" and "longform" are playlists and "comedy" is a Random function between the "music" switch and the "humor" playlist. Problem was, that statement in 2.0.0.-rc1 would throw the following error:

Error 5: this value has type
  _ * source(audio=?A, video=?B, midi=?C)
  .{
    time : () -> float,
    shutdown : () -> unit,
    fallible : bool,
    skip : () -> unit,
    seek : (float) -> float,
    is_active : () -> bool,
    is_up : () -> bool,
    log : 
    {level : (() -> int?).{set : ((int) -> unit)}
    },
    self_sync : () -> bool,
    duration : () -> float,
    elapsed : () -> float,
    remaining : () -> float,
    on_track : ((([string * string]) -> unit)) -> unit,
    on_leave : ((() -> unit)) -> unit,
    on_shutdown : ((() -> unit)) -> unit,
    on_metadata : ((([string * string]) -> unit)) -> unit,
    is_ready : () -> bool,
    id : () -> string,
    reload : (?uri : string?) -> unit,
    length : (() -> int)
  }
but it should be a subtype of the type of the value at aviaryradio.liq, line 61, char 20-46
  _ * _.{selected : _}

I poked and prodded at it and eventually traced it to the "comedy" variable (subbing in "humor" or "music" would not throw the error) but I had no idea what the error was trying to tell me. Out of desperation, I decided to shake up the order of the statements in the switch statement:

special = switch (id="specswitch", [
                    ({1w and 05h-00h}, tracker),                  # MOD MONDAYS!
                    ({5w and 18h-19h}, delay(7200.0, longform)),  # Play a Longform Mix on Friday (don't re-fire within 2 hours)
                    ({3w and 17h-19h}, comedy)                    # Do the Comedy Mix on Wednesdays
                   ])

(swapping the last two lines) and it no longer throws the error. But I don't understand why? Anyone have any ideas?

Thanks for reading this far. Any or all or none of these might be things to change in 2.0.0-rc1 or beyond. They're just things that struck me right off the bat and I wanted to share and get others' insights about them. I'm certainly a newb at Liquidsoap and Ocaml is completely new to me too, so I'm positive there's a lot I could learn. ^_^

[toots]

Hi!

Thank you for this feedback, this is really appreciated and exactly what we needed. Feel free to be yourself and be as verbose as you want!

I'm naturally a little more succinct so...:

1. I have brought back the old playlist IDs!
2. I have brought back the old next playlist command!
3. This looks like a bug. Any chance your could reproduce in isolation so we can confirm that it is indeed one?
4. This is a limitation of the typing system as it currently is. We're actively working on improving it. Matter of fact, this might already been fixed in main, @smimram just pushed to work on this topic recently.

For 4., basically, one of your source has an extra selected method so, it is comes too early in the playlist, we infer that all sources need to have a selected method. However, if a source without this method comes first, we infer that all sources just need the same, more limited methods as this first one and, the source with a selected method is valid, just has more methods than required.

Another way to work around is to subtype all the sources to just sources without methods, which is all that the switch really care about:

special = switch (id="specswitch", [
      ({1w and 05h-00h}, (tracker:source)),                  # MOD MONDAYS!
      ({3w and 17h-19h}, (comedy:source)),                   # Do the Comedy Mix on Wednesdays
      ({5w and 18h-19h}, (delay(7200.0, longform):source))   # Play a Longform Mix on Friday (don't re-fire within 2 hours)
])

Hope this helps. Please, keep us posted about 3., and wether it is a bug or not. Thanks!

[Kerosel]

It does help! I think I understand the typing issue... once I gave it the "selected" item ("comedy") it was expecting more "selected" items, but I gave it a playlist ("longform")? The error makes sense now too in that light: complaining that it had a "playlist" item but was expecting a "selected" item.

I downloaded the latest 2.0.0 .deb and installed it to get around the metadata character crashes (#1960). I see the auto-generated default playlist IDs are back.

I tried out the (playlist).next telnet command and it gives the next queued item in the playlist. The previous versions gave the next dozen items, showed which were queued and if the playlist was active, it tagged the top one as "playing" (I'm making this one up, but it's representative of what it did)...

[Playing]replaygain:/mnt/media/<...>/blues.flac
[queued]replaygain:/mnt/media/<...>/funky.flac
replaygain:/mnt/media/<...>/hiphop.mp3
replaygain:/mnt/media/<...>/rock.flac
[etc etc]

... but I'm trying to think of if I'd ever want or need more than just what's coming up next. I think the longer list appealed to the nerd in me, but maybe the next item is all I really need to know.

Now, on to item 3 with the fallback. I'm doing some testing and trying to think of what other tests I can run, but here's what I discovered thus far:

On Monday morning the script had crashed (from #1960) and I restarted it. According to my "special" switch statement, we were in the middle of MOD Mondays so it was set to play tracker files. Upon starting, it played one standard morning music file and then went on to play tracker files for the rest of the day. I figure this is probably not a bug: the tracker playlist may not have been ready before the standard music lists were (there are 1200+ files in the tracker directories) so it fell back to the standard music until the tracker playlist was ready.

Today, the switch to comedy happened on time: after the song playing past 5:00pm completed, it switched over to the comedy random statement. It then played another song, but that was via the random statement and not because the fallback was playing another song before switching. The logs plainly indicated it had switched over to comedy.

But before that, I was punching songs into the queue in my test script to troubleshoot another little quirk. When I pushed an item into the previously-empty queue, the fallback would take two songs to fall up (forward?) to the queue. If I pushed an item into the queue while the queue was playing, it would stay on the queue and play the next item like I would expect (as opposed to maybe falling back to the playlist between queue items).

I mentioned before that it looked like the queue item was not resolving until the next song played, and this seems to be a pattern I can reproduce:

requests.push replaygain:/mnt/media/audio/tracker/Leviathn/K_SGROOV.IT
2
END
request.trace 2
[2021/09/22 23:00:52] Pushed ["replaygain:/mnt/media/audio/tracker/Leviathn/K_SGROOV.IT";...].
END
request.trace 2
[2021/09/22 23:00:52] Pushed ["replaygain:/mnt/media/audio/tracker/Leviathn/K_SGROOV.IT";...].
END
stream.skip
Done
END
request.trace 2
[2021/09/22 23:00:52] Pushed ["replaygain:/mnt/media/audio/tracker/Leviathn/K_SGROOV.IT";...].
[2021/09/22 23:01:09] Resolving "replaygain:/mnt/media/audio/tracker/Leviathn/K_SGROOV.IT" (timeout 60s)...
END
request.trace 2
[2021/09/22 23:00:52] Pushed ["replaygain:/mnt/media/audio/tracker/Leviathn/K_SGROOV.IT";...].
[2021/09/22 23:01:09] Resolving "replaygain:/mnt/media/audio/tracker/Leviathn/K_SGROOV.IT" (timeout 60s)...
[2021/09/22 23:01:17] Pushed ["annotate:replaygain_track_gain=\"+0.82 dB\":/mnt/media/audio/tracker/Leviathn/K_SGROOV.IT";...].
[2021/09/22 23:01:17] Resolving "annotate:replaygain_track_gain=\"+0.82 dB\":/mnt/media/audio/tracker/Leviathn/K_SGROOV.IT" (timeout 60s)...
[2021/09/22 23:01:17] Pushed ["/mnt/media/audio/tracker/Leviathn/K_SGROOV.IT";...].
END

So it appears fallback is doing it's job, it's the queue that isn't ready until another song plays. Feels to me like pending resolutions aren't resolved until a track change rather than immediately.

This would probably also explain why I see the same behavior in my "album" playlist. It starts out pointed at an empty file with a "watch" reload mode. Once I copy a list of items into the file and the watch updates the playlist, the items are "pushed" but the first one isn't resolved until a track change.

That's my best theory so far. I can drop this into an official issue for easier tracking if you like, or I can keep it here in discussions if it's not a bug or something that doesn't need to be fixed (i.e. purposefully not constantly checking if something needs to be resolved because it's too resource intensive).

And by the way, that quirk I was poking at (mentioned above) seems to be if you have an apostrophe/single quote in the file path, the replaygain protocol silently fails. It's probably blowing up quoting somewhere in the extraction script if I had to guess. I fixed it by getting rid of the apostrophe in a directory name (it was named like "Foo's Music") but could be worth reinforcing the code against such foolishness. ^_~

Cheers!

[Kerosel]

Okay, I'm amending my theory (and probably going to file an official issue) because recent tinkering makes me think sources that were not ready that then become ready don't actually resolve their first item until they're called upon (and they fail that first time because they have nothing ready/resolved to play back).

Short version (I'll try, at least!): just to mix things up I added some files to my standard request queue, which is a 50/50 random with the standard music jukebox. What happened (backed up by the log) is two jukebox songs played, the request queue prepared it's next item, then two more jukebox songs played before the request song played.

My thought is that once an item was pushed to the request queue, it became ready (the random statement will not select failed sources) but when random finally did select it (after two jukebox songs), it had nothing to give but did at least resolve/prepare it's first item. When random came back to it again, it had a resolved item to play.

This explanation also explains what happens with the "upnext" queue and "album" playlist. Once an item is in the upnext queue, it signals ready and fallback does try to hit it on the track change, but the queue has nothing to give and only resolves it's next item while fallback falls back to the jukebox for another song. On the next track change, fallback tries the upnext queue again and gets a hit.

[pranza]

who could have known this shit! i couldn't go to sleep till 3AM because introducing some broadcast to a playlist triggered LS to collapse! i was looking at all these colons and semicolons and brackets to be in place just to find out this - that a certain type of source has to be written below certain other types of source to work! thank you!!!