fix(playout): when shows ends, next shows starts without fade-in/fade-out

[togir2]

Description

Liquidsoap did a harsh cut at the end of every show.
This MR fixes this an (re)enables settings for the crossfading by

• adding the values "default_crossfade_duration", "default_fade_in" and "default_fade_out" from the "preference" page to the StreamPreferences api.
• The new values are then used while generating the liquidsoap entrypoint, and can also be changed via telnet.

Testing Notes

Created a VM, installed my Libretime branch, scheduled some shows to hear the fade at the end of the show.
I also run "make test" in the "playout" folder.

Links

Closes: #2070

https://discourse.libretime.org/t/how-to-fade-out-shows/700
https://discourse.libretime.org/t/no-fadestop-at-the-show-end/1412

[codecov]

Codecov Report

Merging #2412 (076fa18) into main (be3964c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2412   +/-   ##
=======================================
  Coverage   58.60%   58.60%           
=======================================
  Files         146      146           
  Lines        4343     4343           
=======================================
  Hits         2545     2545           
  Misses       1798     1798           

Flag	Coverage Δ
playout	25.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[jooola]

Hey, it is amazing that you spend some time on fixing this! Thanks a lot! Ping me when this is ready for a proper review.

I had a few small question though, did you manage to get it fixed ? Or are you still searching ?

Did you consider that liquidsoap is supposed to read the fade in/out from the play request annotations ?

libretime/playout/libretime_playout/player/liquidsoap_gateway.py

Lines 11 to 40 in 32e0c2a

	def create_liquidsoap_annotation(file_event: FileEvent) -> str:
	# We need liq_start_next value in the annotate. That is the value that controls overlap duration of crossfade.
	annotations = {
	"media_id": file_event["id"],
	"liq_start_next": "0",
	"liq_fade_in": float(file_event["fade_in"]) / 1000,
	"liq_fade_out": float(file_event["fade_out"]) / 1000,
	"liq_cue_in": float(file_event["cue_in"]),
	"liq_cue_out": float(file_event["cue_out"]),
	"schedule_table_id": file_event["row_id"],
	"replay_gain": f"{file_event['replay_gain']} dB",
	}
	# Override the the artist/title that Liquidsoap extracts from a file's metadata
	# with the metadata we get from Airtime. (You can modify metadata in Airtime's library,
	# which doesn't get saved back to the file.)
	if "metadata" in file_event:
	if "artist_name" in file_event["metadata"]:
	artist_name = file_event["metadata"]["artist_name"]
	if artist_name:
	annotations["artist"] = artist_name.replace('"', '\\"')
	if "track_title" in file_event["metadata"]:
	track_title = file_event["metadata"]["track_title"]
	if track_title:
	annotations["title"] = track_title.replace('"', '\\"')
	annotations_str = ",".join(f'{key}="{value}"' for key, value in annotations.items())
	return "annotate:" + annotations_str + ":" + file_event["dst"]

https://www.liquidsoap.info/doc-1.4.4/reference.html#fade.in
https://www.liquidsoap.info/doc-1.4.4/reference.html#fade.out
https://www.liquidsoap.info/doc-1.4.4/reference.html#cue_cut

[togir2]

I had a few small question though, did you manage to get it fixed ? Or are you still searching ?

In my test it did work after applying the patch. Is it not working for you?

Did you consider that liquidsoap is supposed to read the fade in/out from the play request annotations ?

Thanks, I didn't saw that. I will remove the "default_fade_in" and "default_fade_out".

[jooola]

In my test it did work after applying the patch. Is it not working for you?

I didn't test yet, I was just curious about the reason why it was broken and why your changes fixed it.

Thanks, I didn't saw that. I will remove the "default_fade_in" and "default_fade_out".

Hmm, I like your changes about this default_fade_in, default fade out, could you keep them around ? We might want to keep them. But I have to think a bit more about whether it is a good idea.

[togir2]

I didn't test yet, I was just curious about the reason why it was broken and why your changes fixed it.

I do think it was broken because the "duration" for the crossfade was set to 0. I assume the "fade_in" and "fade_out" are supposed to happen during the crossfade "duration".
Also the "smart=true" option does in my observation sometimes decide, a cut is better than a fade.

libretime/playout/libretime_playout/liquidsoap/1.4/ls_script.liq

Lines 25 to 28 in 32e0c2a

	# the crossfade function controls fade in/out
	l = crossfade(duration=0., smart=true, l)

If you want to keep the "default_fade_in" and "default_fade_out" then I would say this PR is ready from my side :)

[jooola]

I do think it was broken because the "duration" for the crossfade was set to 0. I assume the "fade_in" and "fade_out" are supposed to happen during the crossfade "duration".
Also the "smart=true" option does in my observation sometimes decide, a cut is better than a fade.

The fade in/out does not happen on the same queue, this is why there are multiple queues create_source. With that in mind, I am unsure what the crossfade exactly does in the create_source context. But I think its not the only one at play in the transition system.

Also, the fade between shows seemed to be working before (maybe in around the 3.0.0-alpha.* release) and I probably broken something, or a newer version of liquidsoap broke it.

I am still trying to understand why we have multiple request queues. This part of the liquidsoap script was written 7/10 years ago, and I think liquidsoap didn't have all the feature for fading it had back then. So a lot of the timing management happens on the python side.

[togir2]

I did some more experiments and found that replacing the crossfade function with fade.in and fade.out did get the tracks to fade in/out.

Could you please test it again?

I tested it on a Debian11 VM with a fresh install of this branch and the fade did work for tracks in shows as for the end of shows.

This is why I think the crossfade operator is not working here:

In Liquidsoap version 1.4.3. crossfade is implemented as a cross with a custom transition (fade_in and fade_out).
https://github.com/savonet/liquidsoap/blob/9f730f2c5f6195e37a761f7b1eb5a74395d22b18/src/libs/fades.liq#L433-L436

The "duration" argument is passed through to the cross function.
In the implementation of the cross operator the value duration is used to determine how log the crossfade should take.

It is set to the cross_lenght parameter

https://github.com/savonet/liquidsoap/blob/f075905715584bdc236fe2078e26352a5b7f1c5f/src/operators/cross.ml#L30-L34

This can be overwritten with metadata, but the current annotation does not include a "override_duration" field so in our case it is always 0.
https://github.com/savonet/liquidsoap/blob/f075905715584bdc236fe2078e26352a5b7f1c5f/src/operators/cross.ml#L186-L198

So I assume the crossfade is starting to fade.out the track but because the duration is set to 0. the "cross" is completed immediately and the next source of the queue is started. Our queues do only ever contain one track at a time so there is no next source to play.
The next queue is activated and the same happens for the fade.in.

Replacing the crossfade with a fade.in/out removes this time boundary as there is no longer a "cross" function involved.

Until the tag 3.0.0-alpha.8 there was a custom crossfade_airtime function.
In tag 3.0.0-alpha.9 it was replaced with the crossfade function but was unable to find why.
https://github.com/libretime/libretime/blob/ecd302068c443af3eaa44bd3f81f45cdd88b2155/python_apps/pypo/liquidsoap/1.4/ls_script.liq#LL76C9-L76C18

The crossfade_airtime function is basicly a reimplementation of the crossfade function, except that cross has no duration set, so it will use the default value (5.0).

def crossfade_airtime(s)
  #duration is automatically overwritten by metadata fields passed in
  #with audio
  s = fade.in(type="log", duration=0., s)
  s = fade.out(type="log", duration=0., s)
  fader = fun (a,b) -> add(normalize=false,[b,a])
  cross(fader,s)
end

---

Alternatively we could also extend the annotation to include the "liq_cross_duration", but as we are not crossfading in any case I do like it more to use the the explicit fade.out and fade.in functions. Maybe the "Crossfade duration" form from the settings page could be removed as it does not have any affect on the script.

[jooola]

Nice digging! I think what you wrote above should be in the commit body, it is a gold mine for future miners (I'll add it while merging).

I've tested the PR and it is working! We might be loosing some of the "smart" from the crossfade, but since we were using it wrong, I would keep your fix as it is, and rework the whole schedule queues system it in the future.

The "Crossfade duration" is used, we must keep it. I think the legacy app is using it compute the cue_in/cue_out values which in the end has an impact on the crossfade/fade duration:

libretime/legacy/application/models/Scheduler.php

Line 485 in 32e0c2a

private function calculateCrossfades($instanceId)

The problem is that the streaming code is spread between legacy (php code), the new api, the playout app and liquidsoap, and it's difficult to work with. I'm slowly working on improving this, but it takes time...

I'll do some more testing, and let this PR sit for the day.

[jooola]

The only missing part is that we don't have any crossing between shows, only fades. But that's unrelated and for another ticket.

Thanks a lot for the patch.

[esbobkov]

Thanks for The fix!

@jooola Hello!
When do you plan new release with this fix?

[jooola]

When do you plan new release with this fix?

There is no ETA yet, it should be released in 3.1.0. I want to ship some important changes in 3.1.0 so this will probably need some more time.