Bugzilla – Bug 16663
Maybe an easy fix for the very strict fallback alarm
Last modified: 2011-05-19 13:52:15 UTC
Hi Ben, see you are still on default-copy for this category, so hope you read it :D On the forum a very interesting log for a fallback alarm on MySqueezebox.com turned up. See it here and also read the investigation in the following post: http://forums.slimdevices.com/showthread.php?p=589519 In short: While alarm is playing the actual radio stream changes (don't know if this is special for the BBCApplet from triode used or a more common case also for normal radio streams). This leads to a short second where the audio-buffer is 0 -> and the sledgehammer recognized an audio-state of '0' => thus instantly firing the backup alarm. I think such a scenario can be workarounded very very easily: why not (in _alarm_sledgehammerRearm) just increase a variable every time the audio_state!=1 ... and reset back to 0 when audio_state==1. And whenever the variable reached a limit, only then set 'hammer=true'. (Note that the decodeStatePoller currently is set to 10seconds which might be too long to check for the next audiostate. Whenever audio_state !=1 maybe this should be changed to 3 seconds and when audio_state is back at 1 or the fallback fires reset it to 10seconds). Maybe even just a simple boolean 'audioStateSecondCheck' is sufficient. Then the fallback alarm would be a bit more lenient and forgive most simple rebuffering scenarios. Maybe that's simple to implement but has huge improvement for the alarm.
an excellent suggestion. I dislike very much the way we "hammer" too easily and would like to arrive at a saner approach to being defensive about firing the alarm.
though I still think on a real server disconnection it should stay strict (for now)
*** Bug 16190 has been marked as a duplicate of this bug. ***
*** Bug 15948 has been marked as a duplicate of this bug. ***
*** Bug 15998 has been marked as a duplicate of this bug. ***
Another user where fixing this bug would help: https://bugs-archive.lyrion.org/show_bug.cgi?id=16663 (logs are attached in the posting). The RadioStream does not start within a second (according the the user it typically takes 2-3 seconds when he manually starts the stream), so the fallback kicks in.
Discussed increasing to 10 seconds.
Please ignore comment #6, the stream didn't start (timeout) that's why the fallback sounded, was too late when I looked into that log first ;) So if a stream is silent more then 10 seconds (like the intial timeout) the fallback will sound? So you plan to just wait for two decodestate-pollers with audiostate==0 ? Sounds good enough to me. Remember to decrease the decode-state-timeout to 5 seconds, otherwise when checking only every 10 seconds the real dropout could be 19seconds.
Is there anything one could to help fixing this annoying fallback alarm bug? I'd be happy to send in logs, try out patches on my radio or whatever is needed, but I need directions. I'm watching svn.slimdevices.com (trunk and 7.5.*) since a while already and I don't see any alarm-related changes so I didn't dare to try out one of the 7.6/beta releases yet.
Created attachment 7232 [details] patch to back off the sledgehammer and only move to fallback after 6 consecutive audio failures during the alarm With the above patch, I ran the following test cases Test: Server down when configured alarm time hits Expected behavior: after 20 seconds the fallback alarm sounds (same as in 7.5) Observed behavior: works as designed Test: Server up when alarm hits but dies during playback Expected behavior: a serverDisconnected notification during alarm playback should result in an immediate firing of the fallback alarm (same as in 7.5) Observed behavior: works as designed Test: Server in fine shape when alarm hits and stream operates properly Expected behavior: user should never hear the fallback alarm in an everything-is-fine state Observed behavior: works as designed Test: Server up when alarm hits but pointed to a bad stream (tested with an old defunct URL for TSF Jazz, http://player.tsfjazz.com/tsfjazz.pls.php) Expected behavior: AlarmSnoozeApplet will check the decode state every 5 seconds (twice as often then 7.5) and after 6 consecutive failures (30 seconds), only then trigger the fallback alarm. (New behavior for 7.6) I believe the last test exercises the change suggested by Stefan and implemented by my patch here. I'm happy with how it worked and am going to consider checking it in to 7.6/trunk within the next day or so. Comments welcome and appreciated.
== Auto-comment from SVN commit #9419 to the jive repo by bklaas == == http://svn.slimdevices.com/jive?view=revision&revision=9419 == Fixed Bug: 16663 Description: require 6 consecutive failed states for audio decode status (i.e., nothing playing out the speaker) when an alarm is active and not in snooze also, rearrange a few log messages in AlarmSnooze to prevent over-the-top printing of console messages that didn't apply to the local player
lack of comment on this bug is probably because our emailer system for bugzilla has been down for several days. however, I went ahead and checked in the patch because I think it's sane, makes sense, and improves the user experience with alarm. I've tested it thoroughly, but please let me know if anything with alarm seems wrong as a result of this checkin. Stefan, thanks again for the suggestion.
Looked at the patch and it looks good, code review wise (sorry, don't run betas anymore in 'production', my wife would kill me ;) ). Well I guess this unfortunately will only solve the problems like seen with the BBCiPlayer, where the only reason the audio stopped (and returned) was that the stream was changed while alarm was playing. (Or the stream was just buffering) In any other case, once the streams has stopped (due to disconnect or whatever) I guess it won't recover. Still - lets hope it resolves some of the various fallback alarm scenarios.
>In any other case, once the streams has stopped (due to disconnect or whatever) >I guess it won't recover. To be clear, the fallback alarm will trigger right away if there's a server disconnect, which is a very different kind of failure. The scope of this bug was specifically to deal with times where all other parts of the equation were in working order, but no audio was present at alarm time. What this bug fix changes is that it doesn't trigger that fallback on the user immediately after the first fail, but rather needs 6 consecutive failures before triggering the fallback. In other words, it's a step back in intensity of hitting that fallback alarm, but ANY stream that fails during an alarm willm within 30 secondsm trigger the fallback. If the stream comes back during that time, then it will not trigger the fallback.
In order to test this patch, is it enough to install, say, tomorrow's nightly build for 7.6?
Verified on 7.6.32434 with Radio and TOuch fw 9443 \ Fallback was triggered correctly after trying to play the selected alarm sound.