Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repeat Metadata Entry when Smart Crossfade uses Default Action #2153

Closed
Kerosel opened this issue Jan 12, 2022 · 8 comments
Closed

Repeat Metadata Entry when Smart Crossfade uses Default Action #2153

Kerosel opened this issue Jan 12, 2022 · 8 comments

Comments

@Kerosel
Copy link

Kerosel commented Jan 12, 2022

Describe the bug
I recently started tinkering with .php scripting to pull information from LiquidSoap and display it on a page. My first page is using the "<stream>.metadata" command to get metadata from the previous 10 songs. This has led me to notice that when using the smart crossfade function, if the smart crossfade falls through all its decisions and uses the "default" behavior, a song's metadata will be repeated in the "<stream>.metadata" listing.

The metadata shows up as expected when the song begins playback, but will added to the list again when the "default" transition is made, just before the metadata for the next song is added:

Artist Title Album Year Playlist Time Played
  Bounds of Foundry     evening 2022/01/11 22:57:16
Stanton Warriors Night Mover Wipeout Pure 2005 evening 2022/01/11 22:53:29
  Escape from Na Pali   1999-04-11T17:35:16Z evening 2022/01/11 22:51:19
  Escape from Na Pali   1999-04-11T17:35:16Z evening 2022/01/11 22:51:19
Valve Eon Trap Half Life 2: Episodes 1 + 2   evening 2022/01/11 22:48:10

In this case, the transition from "Escape from Na Pali" to "Night Mover" was the smart crossfade default:

2022/01/11 22:53:28 [decoder:2] Decoding "/mnt/media/audio/tracker/Unreal Na Pali/24-ENDING.IT" ended: Ffmpeg_decoder.End_of_file.
2022/01/11 22:53:29 [evening:3] Prepared "/mnt/media/audio/collection/games/wipeout pure/14 - Stanton Warriors - Night Mover.flac" (RID 20).
2022/01/11 22:53:29 [crossfade_0:3] Analysis: -22.484979dB / -22.601413dB (4.96s / 4.96s)
2022/01/11 22:53:29 [crossfade_0:3] No transition: using default.

Any other transition in the smart crossfade does not cause this behavior, nor does it appear if the crossfader is not used. I played a 2+ hour list of songs with no crossfades and the recent metadata list behaved exactly as expected.

To the best of my knowledge this bug is strictly cosmetic, so doesn't have to be a high priority if there are bigger bugs to squash.

To Reproduce
Something super-simple that involves the smart crossfade would probably be adequate for testing:

songs = playlist("/directory/of/songs/")
stream = mksafe(crossfade(smart=true,songs))

Then keep an eye on the "stream.metadata" listing in the telnet/Unix socket interface and look for repeats when the crossfade uses the "default" transition.

Expected behavior
When the smart crossfade punts and falls through to the default behavior, song metadata should not be added again to the list of recent metadata for a stream.

Version details
Debian Linux - Buster
Liquidsoap 2.1.0 (but originally seen in 2.0.0)

Install method
Downloaded Debian Package directly from GitHub Actions and installed manually

@toots
Copy link
Member

toots commented Feb 2, 2022

Hi!

Thanks for reporting. I have made some correction on the switch operator which might have impacted this. Any chance you could try with the latest v2.0.3-preview?

Also worth noting that the current main (version 2.1.0) is in state of change so we won't be prioritizing issues there until some of the ground work has been finished (which depends on new issues being fixed in the stable branch heh)

@Kerosel
Copy link
Author

Kerosel commented Mar 1, 2022

Sorry for the delay, been off on vacation with limited connectivity. Besides, should one really be working on GitHub projects on vacation? Depends on the vacation, I suppose.

I think i saw you mention to someone else that main is too volatile for debugging purposes. With that in mind I went back to 2.0.0 a few weeks ago. Now I've gone to the 2.0.3-preview for troubleshooting this problem.

The problem is now... different. It looks like when the smart crossfade does a default transition, the metadata list gets a double-entry of the song that's finishing and the song that's starting never makes the list at all.

Artist Title Album Year Playlist Time Played
Frank Klepacki Smash Command & Conquer: Red Alert 1996 day 2022/03/01 11:52:20
Valve Self Destruction Half Life 2: Episodes 1 + 2   day 2022/03/01 11:45:43
Valve Self Destruction Half Life 2: Episodes 1 + 2   day 2022/03/01 11:45:43
Atari Games The Rock (Remix) Rush 2049   day 2022/03/01 11:42:35

The actual list of songs was The Rock (crossed) Self Destruction (default) Speedster (crossed) Smash. Self Destruction made the list twice, Speedster doesn't appear at all.

2022/03/01 11:42:35 [day:3] Prepared "/mnt/media/audio/MP3s/Rush 2049/08 - Atari Games - The Rock (Remix).mp3" (RID 5).
2022/03/01 11:42:35 [crossfade_0:3] Analysis: -22.191490dB / -18.161010dB (4.96s / 4.96s)
2022/03/01 11:42:35 [crossfade_0:3] new >= old + margin, old >= medium and new <= high.
2022/03/01 11:42:35 [crossfade_0:3] New source is significantly louder than old one.
2022/03/01 11:42:35 [crossfade_0:3] Transition: crossed, fade-out.
2022/03/01 11:45:42 [decoder:2] Decoding "/mnt/media/audio/MP3s/Rush 2049/08 - Atari Games - The Rock (Remix).mp3" ended: Ffmpeg_decoder.End_of_file.
2022/03/01 11:45:43 [day:3] Prepared "/mnt/media/audio/MP3s/Orange Box/Half-Life 2 Episode 1 & 2/Self Destruction.mp3" (RID 4).
2022/03/01 11:45:43 [crossfade_0:3] Analysis: -infdB / -26.284505dB (4.96s / 4.96s)
2022/03/01 11:45:43 [crossfade_0:3] new >= old + margin, old <= medium and new <= high.
2022/03/01 11:45:43 [crossfade_0:3] Do not fade if it's already very low.
2022/03/01 11:45:43 [crossfade_0:3] Transition: crossed, no fade.
2022/03/01 11:47:38 [decoder:2] Decoding "/mnt/media/audio/MP3s/Orange Box/Half-Life 2 Episode 1 & 2/Self Destruction.mp3" ended: Ffmpeg_decoder.End_of_file.
2022/03/01 11:47:38 [day:3] Prepared "/mnt/media/audio/collection/games/ridge racer/08 - NAMCO SAMPLING MASTERS - Speedster [I Like AT Mix].flac" (RID 6).
2022/03/01 11:47:38 [crossfade_0:3] Analysis: -21.873140dB / -25.580788dB (4.96s / 4.96s)
2022/03/01 11:47:38 [crossfade_0:3] No transition: using default.
2022/03/01 11:52:20 [decoder:2] Decoding "/mnt/media/audio/collection/games/ridge racer/08 - NAMCO SAMPLING MASTERS - Speedster [I Like AT Mix].flac" ended: Ffmpeg_decoder.End_of_file.
2022/03/01 11:52:20 [day:3] Prepared "/mnt/media/audio/collection/games/red alert/15 - Frank Klepacki - Smash.flac" (RID 0).
2022/03/01 11:52:21 [crossfade_0:3] Analysis: -infdB / -25.313312dB (4.96s / 4.96s)
2022/03/01 11:52:21 [crossfade_0:3] new >= old + margin, old <= medium and new <= high.
2022/03/01 11:52:21 [crossfade_0:3] Do not fade if it's already very low.
2022/03/01 11:52:21 [crossfade_0:3] Transition: crossed, no fade.

Hope this helps provide new clues.

@Kerosel
Copy link
Author

Kerosel commented Mar 2, 2022

Looking things over later, I have more information. Before a default transition, the currently playing song shows up in the metadata list when it plays (as it should). Then after the transition it shows up again, rather than the next (now current) song. I think before in 2.0.0 this same thing was happening only the current song was then being immediately put into the list after the second entry of the previous song.

Also, this affects the metadata push (ICY-INFO?) so that the player shows the wrong (previous) song information after a default transition. This may have always been the case but because the current song was pushed to the list after the re-push of the previous song, it was pushed out over the stream as well and the player did show the currently playing song. Therefore it wasn't immediately apparent that something weird was going on.

@Kerosel
Copy link
Author

Kerosel commented Mar 9, 2022

Well, hopefully I'm not wearing out my welcome on this thread but I keep finding more little things that may or may not help with finding the fix... figure it can't hurt to write it up.

I've noticed that the smart crossfade default action (that's causing this metadata mishap) is cutting off the start of the "next" song. Almost as if it was trying to overlap/crossfade them but the audio isn't switching over to the next song until the previous one is finished. Basically the ending song plays to completion and the next song stars a few seconds into the song already.

What I noticed tonight is that while the current song is not getting added to the metadata list or being pushed out on the stream, the "request.on_air" socket command returns the proper RID which can then be used in "request.metadata" to get the current song's metadata. So the system is aware of what song is playing and the metadata for it can be fetched. There's just a fumble going on in the metadata push to the stream and it being added to the "<stream>.metadata" listing.

@toots
Copy link
Member

toots commented Mar 21, 2022

You're never wearing out a welcome when adding relevant feedback! Testing this rn.

toots pushed a commit that referenced this issue Mar 23, 2022
… sure crossfade transition sources do no re-send old source metadata.

Fixes: #2153
@toots toots closed this as completed in 43b0c4d Mar 23, 2022
@toots
Copy link
Member

toots commented Mar 23, 2022

I just had a review of the code. We do re-insert the ending track's metadata when computing the crossfade transition. This is because the fade operators rely on metadata to adjust their duration and etc.

This, indeed, leads to metadata appearing twice in the same track and I can understand how this can create confusion.

I've just pushed some changes that de-duplicate metadata coming out of the crossfade. This could generate change in existing behavior but I'm suspecting the de-duplicated source is the expected behavior for most people. I added an option to disable it in the crossfade, if needed.

@Kerosel
Copy link
Author

Kerosel commented Apr 14, 2022

Finally got around to loading up 2.0.4-preview and checking this out. I don't know if you get notifications on closed issues, but I'll put this here anyway. Also I have a possible clue or lead at the very end, feel free to skip there directly.

So the duplicate metadata is gone now, but the "slot" that the repeat metadata was in is blanked out, which leaves a hole in the list. I.E.:

Artist Title Album Year Playlist Time Played
Namco Deep Resonance R: Racing Evolution 2003 evening 2022/04/13 23:04:35
福井健一郎 相克 [CONFLICT] —Stage5 #3 カタパルト施設— Einhänder Original Soundtrack 1997 evening 2022/04/13 23:01:28
谷岡久美 覚悟を決めろ “FINAL FANTASY CRYSTAL CHRONICLES” Original Soundtrack 2003 evening 2022/04/13 23:00:37
           
Simon Wiklund We Have a Situation (The Emergency) Bandits: Phoenix Rising 2003 evening 2022/04/13 22:53:49
Namco Select 1 R: Racing Evolution 2003 evening 2022/04/13 22:50:26
Tetsuya Shibata Main Menu =garage life= Auto Modellista Original Soundtrack (Disc 2) 2002 evening 2022/04/13 22:47:32

The output from radio.metadata is:

--- 6 ---
genre="Techno"
replaygain_album_peak="1.00000000"
rid="6"
on_air="2022/04/13 22:53:49"
replaygain_track_gain="-7.99 dB"
replaygain_album_gain="-6.63 dB"
discnumber="2"
replaygain_reference_loudness="89.0 dB"
status="playing"
initial_uri="/mnt/media/audio/collection/games/bandits/205 - Simon Wiklund - We Have a Situation (The Emergency).flac"
tracknumber="5"
source="evening"
temporary="false"
filename="/mnt/media/audio/collection/games/bandits/205 - Simon Wiklund - We Have a Situation (The Emergency).flac"
title="We Have a Situation (The Emergency)"
decoder="FFMPEG"
artist="Simon Wiklund"
kind="{audio=pcm(stereo),video=none,midi=none}"
on_air_timestamp="1649915629.00"
disc="2"
date="2003"
album="Bandits: Phoenix Rising"
replaygain_track_peak="1.00000000"
--- 5 ---

--- 4 ---
album_artist="谷岡久美"
replaygain_album_peak="0.96505737"
rid="1"
catalognumber="PCCG-00613"
on_air="2022/04/13 23:00:37"
albumartist="谷岡久美"
replaygain_track_gain="-1.00 dB"
replaygain_album_gain="-5.35 dB"
discnumber="1"
replaygain_reference_loudness="89.0 dB"
status="playing"
arranger="Arranger|岩崎英則|Arranger|谷岡久美|Mastering|中里正男|Producer|谷岡久美|Programming|岩崎英則|Recording|赤川新一|Arranger|谷岡久美|Producer|谷岡久美"
initial_uri="/mnt/media/audio/collection/games/ffcc/123 - 谷岡久美 - 覚悟を決めろ.flac"
tracknumber="23"
source="evening"
temporary="false"
publisher="Leafage"
filename="/mnt/media/audio/collection/games/ffcc/123 - 谷岡久美 - 覚悟を決めろ.flac"
title="覚悟を決めろ"
releasecountry="JP"
decoder="FFMPEG"
artist="谷岡久美"
kind="{audio=pcm(stereo),video=none,midi=none}"
on_air_timestamp="1649916037.00"
disc="1"
date="2003"
album="“FINAL FANTASY CRYSTAL CHRONICLES” Original Soundtrack"
composer="谷岡久美"
replaygain_track_peak="0.88528442"
--- 3 ---
album_artist="福井健一郎"

So slot 5 was the repeated metadata and is now empty. But my parsing code uses that number header to create the array index. It creates index 5 but then has nothing to put in it. I could skip it, but then the usual list of the last 10 songs would only have 9 songs in it.

Maybe this is taking up way too much time for a cosmetic/accounting item. I kinda feel like it is, or that I'm being picky. Maybe I'll just never be happy. :P

There is one thing I thought of though: this only happens with the default transition and not any of the crossfades. And all the crossfades use add(), but the default is sequence(). So is there something in sequence() that would repeat metadata that isn't in add()?

@Kerosel
Copy link
Author

Kerosel commented Apr 16, 2022

Found a workaround that works pretty well, it's outlined in Discussion #2339.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants