-
-
Notifications
You must be signed in to change notification settings - Fork 132
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
"Cue out" causing unexpected track ending (Ffmpeg_decoder.End_of_file) #3610
Comments
Thanks for reporting this. I'm gonna do my best to have it fixed soon for a stable release. It would really help to have any kind of minimal reproduction script and tracks! I can see a lot of the details (thanks for the hard investigation work!) but it's still hard to piece together a clue. My guess is that the beginning of the next track is eaten by whatever is happening between cue-out and cue-in. The exception that you are seeing is not an error per say but it indicates that the decoder has reach the end of the files while in the other cases, it moves to the next track without a full decode, which gives us a clue about the parts of the code that are involved when the issue occurs. Since you've worked a lot of this, @RM-FM perhaps you might be able to write a short minimal script that can reproduce it? |
Also, some detailed logging around the issue would be great! Something with log level set to 4.. |
Okay I've done more investigations. In the simple reproduction case that I have tried, I haven't been able to reproduce the issue but I have noticed another issue when the cue cue it too far from the end of the track: the source does not have a request queued to use for crossfade computation. I'm not sure if that could be the issue but setting |
Hi @toots, @RM-FM!) I have a feeling that this The sound at the end of fade_out is similar to sound that sometimes can be heard when you push the stop button on some players during playback. No idea if its connected but usually [cross:3] Analysis: -53.887899dB / -53.090761dB Some logs. Also I have always considered In this section of logs there have been no update of Title - Artist and metedata for the Logs_1
I'm not sure how to reproduce this issue.
So this could again result in pop/clicks, right? |
And here are logs where I've heard pop/click — just in the end of Logs_2
|
Another pop/click between Logs_3
Also tried to reproduce (but couldn't) with:
and test.m3u with different parametres:
But everything ok on `Liquidsoap 2.2.4+git@295a01e |
@toots @gAlleb Indeed, it seems that the meta data update issue is gone. Unfortunatel, I still hear some clicks/pops sometimes... Below log shows the relevant transition between jingle and song.
|
@toots @gAlleb Ups, now I changed the cue values of the jingles in order to test something else and the missing meta data issue came back:
|
Ok thanks. I've been giving this one some thoughts and, even if I don't like doing this, I'm tempted to push a breaking change to fix this. The reason is that Likewise, I haven't confirmed it yet but I'm suspecting that the initial seek or the abrupt track abort might create situations with audio content changes that are too abrupt and become audible. The cue cut operations should be pushed under the playlist so that they play nicely with its logic. I don't think that there's a lot of sense to keep fixing the current paradigm, this would require some pretty bad hacks. I'm gonna see what I can do in the context of a stable release. What I would like to do is make the playlist aware of the cue cut metadata and be able to do the cuts itself. Then, what most scripts would have to do is simply remove the This would apply to all file-based operators, What do y'all think? |
Hi @toots! Thanks. Passing the cue cut function to a playlist seems nice. So all file-based operators would directly read metadata from annotations instead of using cue_cut operator (unless we pass an argument to disable it)? Have you been able to reproduce the pop/click issue with |
Those are all very valid points @gAlleb. However, we already do have an issue at hand: when the cut is too far from the end of the track, the mechanism to fetch the next track gets tripped out and the cut out happens without a track being ready next. |
Hi all! I pushed a PR with a first version of the changes. Would anyone be able to test this build here: https://github.com/savonet/liquidsoap/actions/runs/7455151871 ? |
Hi @toots ! Thanks!
|
That seems unrelated.. Do you have more logs? |
Logs
|
Have you checked the maximum of sources allows in your icecast configuration? |
MAX 20. Active 2. |
Hoo probably related to a recent change. Try adding |
Everything's ok. Everything is cut without cue_cut :) With songs of everage duration 3 minutes and liq_cue_out=10.00 — metadata changes. It needs to be tested with Azuracast too. Should ask @BusterNeece to add release when ready. Perhaps you've eliminated pop/click issue as well. Thanks @toots! Logs
|
Glad to hear. I'm gonna merge this first then we can see. With these changes, requests behave just as if files where the actual cut version of the cue-in/cue-out so I suspect, yes, pop/clicks should be gone if they weren't happening on regular files! |
Yep. Thing is that with Azuracast I can hear pop/clicks sometimes on "regular files". But can we call them "regular"? Azuracast annotates all files with P.S. And I've tested it — and probably |
@gAlleb I think we found the issue with the |
Hi @toots! Everything's fine. Script works without |
Awesome, thank you! |
I though that since no Let's just hope then that there won't be any memory leak. Transitions are smooth now. So 2.2.4 and I hope - 2.3.0 in nearest future, yeah?:) (Coverart memory leak should be gone too in 2.3.0?) Thank you, @toots, for all the hard work! |
I'm working on the memory leak. I think this should be taken care of. Just got to pull from the buffer only when it grows big enough, skipping pulling from the source for the current streaming cycle. |
@gAlleb Cover art is not really a memory leak as far as I've been able to see, just too much increased memory. I have a solution in the work tho: avoid storing cover in memory and use a file mmap instead. I think we should have it for The largest feature for |
Good to hear that things are progressing and evolving. Thanks again, @toots! |
So nice to hear those smooth transitions finally! There wasn't a single transition that didn't fit. Even transitions with very short crossfades (0.0 to 0.2) all sounded perfectly fine. I've tested all this with a standard Azuracast setup and crossfade method "Normal Mode" ( Thanks a lot for the hard work, @toots! |
@toots just had a missing meta data / title information update for a track. Will check and provide logs later… |
Please find the related log data below... Relevant transition was:
This happend twice now. I took note of it already yesterday (different tracks) and today morning once again. Not sure whether this is related to the latest buffer/muxer fixes or not. But I think it's worth to have a closer look. In case it is not related to the latest fixes, I will create a new bug. However, this happend with
|
@toots @gAlleb I'm able to reproduce it each time with the "Burak Yeter - Tuesday" song, independently from the next song. It appears to be related to crossfade and fade out values of 0.0 (but not for all tracks). Changing them to 0.1 fixes the problem. Burak Yeter - Tuesday (Feat. Danelle Sandoval) [Heyhey Remix].mp3
As soon as I set crossfade and fade out to 0.1, meta data update for the upcoming track is working again.. We have many other tracks where crossfade and fade out is set to 0.0 but meta data update is working perfectly fine there (e.g. all jingles). I know that "zero" crossfades should be prevented or processed with a |
Thanks for following up. I can confirm the issue. It might have been introduced by the new "common buffer" crossfade computation. Nonetheless, I am considering an empty crossfade duration as a programming error -- when using crossfade, if you want to avoid overlapping tracks, just return a sequence in the transition. I have pushed a workaround here: https://github.com/savonet/liquidsoap/actions/runs/7832004165 it will set a minimal crossfade duration of one frame and issue a critical log warning when crossfade duration is Let me know if that helps for you. |
Actually a better way perhaps to achieve disabling crossfade without touching the transition code in |
@toots Thanks, will test... In between I used a custom crossfade callback. But it obviously doesn't even touch the callback as there is no I will test this also with the new build now. Let's see how it behaves now..
|
@toots @gAlleb I tested with both Crossfade callback works as expected for both. I can see the expected Liquidsoap 2.2.5+git@9f96e42 Liquidsoap 2.3.0+git@31a2dba |
Ok thanks. Do you have specific crossfade parameters I could use to test again? |
Unfortunately, also Meta data was missing for "GOODBOYS - Future" after the promo "Werbung App - Dance".
|
I think most the issues are related to either crossfade duration 0.0 and fade out 0.0. But it is not really consistent. |
Ok. Would you mind testing the recommended way, which should be to set |
Well, that is not possible with the Azuracast setup. Also, from a usability point of view I do not really like this, tbh. A radio station usually maintains cue and crossfade points individually per track. Not having a crossfade for some of them is a valid case as there are songs/tracks that can‘t / shouldn‘t be crossfaded. That is usually true for songs with abrupt ending. I understand that „no crossfade“ and cross/cross.simple are actually not compatible from a technical point of view. But all those different crossfade and cue settings need to pass the same LS script. From that point of view I would see a crossfade value of 0.0 as a valid one, even if this might be different from a technical aspect. Wouldn‘t it be the most user friendly approach to handle this inside the crossfade methods just like a sequence, without the need of custom callbacks etc? |
Well what I am saying is that setting this:
Should be the right way to achieve no fade out. There's an explanation of how this works here: https://www.liquidsoap.info/blog/2024-02-07-liquidsoap-v2.2.4/ Here's a graph of this specific case: From a sytem point of view, this is a much better situation: this makes it possible to maintain a sane buffer size (anything between maybe |
If I got you right, then also the next track needs to be delayed (fade-in delay)? That would require Azuacast (or any other playout logic) to know that the previous track had a delay set. It might be only me, but my gut feeling is not a good one on that… @gAlleb @BusterNeece What about you? |
@RM-FM, well, I think you got your point and it's understandable. As for the rest. Crossfading requires more or less:
If it is the first case - then one can use even "fade_in=3.". Why not? Sounds interesting :) If it is the second case then next point -->
We however now have a ton of options it's easy to get lost in them and they requires some "tea-drinking-RTFM-sitting" :) At this point I'm very pleased that we have so much options and no "clicks" where they shouldn't be. What is ideal: make annotation of all tracks with cue_in and cue_out and loose the crossfades whatsoever. But then we might have clicks on cue_out (in this case it is quite understandable). Just how it is.
@Moonbase59 has came up with a segue-like example here AzuraCast/AzuraCast#6252 (comment) with a link to "segue" thread on one forum https://yabb.jriver.com/interact/index.php?topic=44605.0 |
@toots Don‘t get me wrong, please. I‘m perfectly fine with having a custom crossfade callback for sequence transitions in place. It just would be great if meta data would not be affected by the type of transition. We have smooth transitions now without any click/pop. Meta data update is working much more reliable than before. There seems to be only a very, very small gap somewhere. I‘m not even sure if it is only crossfade/sequence related - but most likely. However, there was so much improvement, that we will try to overcome those few meta data gaps somehow and put it into production. Thank you for the hard work! |
I took notes of some clicks today while testing Liquidsoap 2.2.5+git@9f96e42. Is it possible that Add workaround to 0. crossfade duration. is causing that? If so, we should revert that I guess. The missing meta data problem is an edge case for 0.0 crossfades only and should not bring back clicks/pops under any circumstances. I'm currently testing with official 2.2.4 and crossfade duration set to minimum 0.1 for all of our tracks. Same for fade out. This works perfectly fine and solved, as expected, the issue with the meta data edge case. The only thing I'm still struggling a bit with, are the 0.1 fade in's on some tracks. I'm playing around here with a customer crossfade callback now using the fade curve and delay as well. What I'd like to achieve is basically this: |
Gives: |
@RM-FM Please let me know if you have a way to reproduce the clicks on |
So, yeah, moving forward, I would recommend keeping crossfade duration to a fixed value unless really required, for instance for a short track coming up, and adjusting the crossfade relative positions using This should also allow to implement @Moonbase59 smart dB trigger to precisely define where the fades should cross: #3701 |
Clicks were caused by The meta data update still doesn't seem to be fixed entirely. It is still missing sometimes. I do not yet fully understand when and why... |
Finally, I was able to reproduce it... Please take note of the line Please find the log of a case where the meta data update failed (Choices => Cooler Than Me).
|
Yeah we identified the problem with @Moonbase59, we have to do a hotfix re-release of |
Confirm that 2.2.4 Hotfix also solved the issue I took note of. However, I'll stay with 0.1 crossfading without fade out, instead of a sequence. It just sounds smoother :-) |
Describe the bug
It appears that
liq_cue_out
values that are set close to the actual track ending causing often an unexpected track end. This can be seen asFfmpeg_decoder.End_of_file
in the log. The same happens after very short tracks like Jingles (e.g. 4 seconds) even without anyliq_cue_out
set.As a result this causes two problems:
Furthermore, missing meta data updates are causing serious subsequent problems. E.g. Azuracast doesn't retrieve the feedback once a track has been played. This might affect artist/song rotation rules etc.
Please also see related AzuraCast discussion:
AzuraCast/AzuraCast#6778
To Reproduce
Use
cue
andcross
(crossfade) in the test script. Setliq_cue_out
to either a value that is very close to the actual song duration or choose a value that is cleary higher that the duration. Setliq_fade_out
andliq_cross_duration
to the same value (e.g. 3.0). Alternatively you should be able to force the behavior also by using a very short track like a jingle (4 seconds, no cue out set)You should see the
Ffmpeg_decoder.End_of_file
error/warning in the log and take note that the meta data stuck on the "faulty" track but do not update. Depending on the tracks you might hear a click/pop at the end of the transition as well.Expected behavior
liq_cue_out
values in a safe manner to avoid unexpected track endingsVersion details
Install method
Azuracast Docker
The text was updated successfully, but these errors were encountered: