Home of the Squeezebox™ & Transporter® network music players.
Page 6 of 12 FirstFirst ... 45678 ... LastLast
Results 51 to 60 of 120
  1. #51
    Senior Member
    Join Date
    May 2010
    Location
    London, UK
    Posts
    802
    Quote Originally Posted by slartibartfast View Post
    Is a fix for this issue possible?
    My uneducated guess is that ‘a fix’ might be a step too far, given all the various ways and places in which the counter is used. So it will be interesting to see if someone comes up with something.

    My next uneducated guess is that maybe a ‘mitigation’ is possible, whereby the Radio is restarted pre-emptively at a time or in conditions of the user’s choosing. In my case, perhaps when the Radio is on standby, between three and four in the morning. I’m not sure how this plays together with alarm settings, or the ‘random’ restarting of stream playback, etc., that has been surprising some users.

    Just idle thoughts.

  2. #52
    Senior Member Steevee28's Avatar
    Join Date
    Feb 2010
    Location
    Mannheim, Germany
    Posts
    103

    Thoughts about a possible fix

    Quote Originally Posted by slartibartfast View Post
    Is a fix for this issue possible?
    I already thought some time about a fix, but it is really not that easy. Basically I only see two options:

    1. fix jiveL_get_ticks(), see https://github.com/Logitech/squeezep...amework.c#L967
    or
    2. fix all lines of code that directly compare timer values.


    Regarding option 1:

    This would include to change the type of returned value of jiveL_get_ticks() from lua_Integer to lua_Number, meaning that it would be required to extend the (missing) upper bits of the jiffy timer value before passing it on to Lua. Checking if the upper bits need to be increased, compared to the value previously returned, would require the storing of the last returned value in some variable, which in turn would either require a multi-thread locking in some way, usage of thread-local-storage or, which would probably be best, the use of interlocked intrinsics. (This is not required, of course, if the Jive framework is single threaded. i don't know that.)
    For some time, facing the notable amount of buggy code lines directly comparing timer values, I thought that this may be a good option. But digging some further in the code, I saw eg. code of process_event() in the jive framework that is storing jiffy timer values in some JiveEvent structure.
    Thus, changing the retured datatype of jiveL_get_ticks() would require to also change the datatype of timer values in structures like that, otherwise the adaption could break further code (e.g. comparing event timestamps to now - don't know whether there is such code).

    Regarding option 2:

    As it was possible to find a notable amount of such buggy code lines in a relatively short amount of time, I guess that there are much more similar lines of code that would require fixing. I'm unfortuantely not completely sure how to fix them all, because I don't exactly know how Lua handles 2-complement lua_Integer values in the wrapping case. In C, I would know what to do, see my post #7, namely only work on timestamp differences.
    As already pointed out, one would also have to pay special attention to code lines like this: framedue = framedue + framerate, see https://github.com/Logitech/squeezep...ework.lua#L353, because, depending on Lua's handling of datatypes, this may result in values that are out of range of the actual timer values.
    In fact the only valid arithmetic operation on timestamps (that are prone to wrapping) should be subtraction from each other. (For that reason, in the company I'm working, we wrapped timestamp values by a special datatype only allowing testing of equality of two timestamps and subtraction from each other, which is of course also dependent on 2-complement arithmetic rules.)

    Anyway, I now believe that option 2, thus fixing all these lines of code is the better option to finally get rid of the problem. But this looks like a pretty bunch of work.

    I'm currently still too far away from enough knowledge about Lua and the Jive framework to be able to do this :-/
    Last edited by Steevee28; 2020-03-06 at 01:52.
    1x Squeezebox Classic, 3x Radio, 1x Touch, LMS 7.9.1 running on ODROID-U3, Ubuntu 16.04 and I'm happy with it! :)

  3. #53
    Senior Member
    Join Date
    Jan 2010
    Location
    Hertfordshire
    Posts
    6,853
    Quote Originally Posted by Steevee28 View Post
    I already thought some time about a fix, but it is really not that easy. Basically I only see two options:

    1. fix jiveL_get_ticks(), see https://github.com/Logitech/squeezep...amework.c#L967
    or
    2. fix all lines of code that directly compare timer values.


    Regarding option 1:

    This would include to change the type of returned value of jiveL_get_ticks() from lua_Integer to lua_Number, meaning that it would be required to extend the (missing) upper bits of the jiffy timer value before passing it on to Lua. Checking if the upper bits need to be increased, compared to the value previously returned, would require the storing of the last returned value in some variable, which in turn would either require a multi-thread locking in some way, usage of thread-local-storage or, which would probably be best, the use of interlocked intrinsics. (This is not required, of course, if the Jive framework is single threaded. i don't know that.)
    For some time, facing the notable amount of buggy code lines directly comparing timer values, I thought that this may be a good option. But digging some further in the code, I saw eg. code of process_event() in the jive framework that is storing jiffy timer values in some JiveEvent structure.
    Thus, changing the retured datatype of jiveL_get_ticks() would require to also change the datatype of timer values in structures like that, otherwise the adaption could break further code (e.g. comparing event timestamps to now - don't know whether there is such code).

    Regarding option 2:

    As it was possible to find a notable amount of such buggy code lines in a relatively short amount of time, I guess that there are much more similar lines of code that would require fixing. I'm unfortuantely not completely sure how to fix them all, because I don't exactly know how Lua handles 2-complement lua_Integer values in the wrapping case. In C, I would know what to do, see my post #7, namely only work on timestamp differences.
    As already pointed out, one would also have to pay special attention to code lines like this: framedue = framedue + framerate, see https://github.com/Logitech/squeezep...ework.lua#L353, because, depending on Lua's handling of datatypes, this may result in values that are out of range of the actual timer values.
    In fact the only valid arithmetic operation on timestamps should be subtraction from each other. (For that reason, in the company I'm working, we wrapped timestamp values by a special datatype only allowing testing of equality of two timestamps and subtraction from each other.)

    Anyway, I now believe that option 2, thus fixing all these lines of code is the only option to finally get rid of the problem. But this looks like a pretty bunch of work.

    I'm currently still too far away from enough knowledge about Lua and the Jive framework to be able to do this :-/
    @mrw suggested a managed reboot at a set time. Would that be possible?

    Sent from my Pixel 3a using Tapatalk

  4. #54
    Senior Member Steevee28's Avatar
    Join Date
    Feb 2010
    Location
    Mannheim, Germany
    Posts
    103
    Quote Originally Posted by slartibartfast View Post
    @mrw suggested a managed reboot at a set time. Would that be possible?
    Of course, that should be possible, but that feels like a bit unsatisfactory solution to me. Especially, there are Squeezeplay applications that do not run on SqueezeOS, like PiCorePlayer. These would still crash after 24.8 days.
    Last edited by Steevee28; 2020-03-06 at 03:11. Reason: Fixed typo 22.8 -> 24.8
    1x Squeezebox Classic, 3x Radio, 1x Touch, LMS 7.9.1 running on ODROID-U3, Ubuntu 16.04 and I'm happy with it! :)

  5. #55
    Senior Member
    Join Date
    Jan 2010
    Location
    Hertfordshire
    Posts
    6,853
    Quote Originally Posted by Steevee28 View Post
    Of course, that should be possible, but that feels like a bit unsatisfactory solution to me. Especially, there are Squeezeplay applications that do not run on SqueezeOS, like PiCorePlayer. These would still crash after 22.8 days.
    I didn't realise this affected PiCorePlayer as well. I am currently waiting for my Controller to reboot in around 3 hours time (24 days 17 hours uptime at the moment). I know this issue has always been there and I could probably count the number of times it has noticeably affected me on the fingers of one hand but knowing that it isn't random but a firmware design error somehow makes it more annoying.

    Sent from my Pixel 3a using Tapatalk

  6. #56
    Senior Member
    Join Date
    Jan 2010
    Location
    Hertfordshire
    Posts
    6,853
    Quote Originally Posted by Steevee28 View Post
    Of course, that should be possible, but that feels like a bit unsatisfactory solution to me. Especially, there are Squeezeplay applications that do not run on SqueezeOS, like PiCorePlayer. These would still crash after 22.8 days.
    I imagine that for a PiCorePlayer a Cron job could perform a reboot once a week.

    Sent from my Pixel 3a using Tapatalk

  7. #57
    Senior Member Steevee28's Avatar
    Join Date
    Feb 2010
    Location
    Mannheim, Germany
    Posts
    103
    Quote Originally Posted by mrw View Post
    My uneducated guess is that ‘a fix’ might be a step too far, given all the various ways and places in which the counter is used. So it will be interesting to see if someone comes up with something.

    My next uneducated guess is that maybe a ‘mitigation’ is possible, whereby the Radio is restarted pre-emptively at a time or in conditions of the user’s choosing. In my case, perhaps when the Radio is on standby, between three and four in the morning. I’m not sure how this plays together with alarm settings, or the ‘random’ restarting of stream playback, etc., that has been surprising some users.

    Just idle thoughts.
    When I wrote my answer I hadn't read your post (did not press the update button in the browser...).

    Why uneducated? ;-) Come on.


    - I totally agree to your opinion regarding a 'fix', which, as I also pointed out in my post, might be a very large step.

    - Your thoughts on 'mitigation' are very good! If you hadn't wrote that already, I now would have asked what the advantage of a managed reboot is over a watchdog reboot. I didn't had that advantage in mind, but now I understand. Thank you.
    Last edited by Steevee28; 2020-03-06 at 03:28.
    1x Squeezebox Classic, 3x Radio, 1x Touch, LMS 7.9.1 running on ODROID-U3, Ubuntu 16.04 and I'm happy with it! :)

  8. #58
    Senior Member
    Join Date
    Jan 2010
    Location
    Hertfordshire
    Posts
    6,853
    Bingo. My Controller rebooted at 24 days 20 hours 31 minutes.

    Sent from my Pixel 3a using Tapatalk

  9. #59
    Senior Member
    Join Date
    Jan 2010
    Location
    Hertfordshire
    Posts
    6,853
    Someone suggested using SSH to perform an auto reboot in this post.

    https://forums.slimdevices.com/archi.../t-104028.html

    Not ideal.

    Sent from my Pixel 3a using Tapatalk

  10. #60
    Senior Member
    Join Date
    May 2010
    Location
    London, UK
    Posts
    802
    Quote Originally Posted by Steevee28 View Post
    Why uneducated?
    Because I have only very lightly dabbed at two pieces of relevant code: the task scheduler and the source of the timer.

    Your thoughts on 'mitigation' are very good!
    Thank you. It might be reasonably easily achieved:

    An Applet could be written to implement a pre-emptive reboot (a timed "Quit", I imagine, which I think will trigger the watchdog).

    Tasks:

    • Identify what "conditions of the userĺs choosing" might be practically implemented and offered in an easily digestible configuration menu,

    • Understand the interaction between rebooting, existing alarm settings, and playback state, and

    • Ensure that relevant state is appropriately saved prior to the reboot, and restored when the Radio starts up.

    Perhaps that just ends up overcomplicating matters. I don't have sufficient knowledge to know what needs to be known.

    A simpler approach might be just to reboot the first time, say, 3.00am is encountered after, say, 20 days of uptime.
    But does that really "solve" the problem and its consequences ? It may be good enough.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •