Bug 1854 - Remote streaming RETRY_TIME could be adjusted to match bitrates and/or OS pipe buffer size
: Remote streaming RETRY_TIME could be adjusted to match bitrates and/or OS pip...
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Streaming To SlimServer
: 6.1.0
: All Linux (other)
: P2 enhancement (vote)
: Future
Assigned To: Blackketter Dean
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-07-22 08:55 UTC by Adrian Smith
Modified: 2009-09-08 09:26 UTC (History)
1 user (show)

See Also:
Category: ---


Attachments
Possible patch (2.51 KB, patch)
2005-07-23 15:53 UTC, Adrian Smith
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Smith 2005-07-22 08:55:12 UTC
On linux 2.4 remote streams which are decoded to wav/flac don't work 
correctly.  This is due to RETRY_TIME in Slim/Web/HTTP.pm being too long for 
the size of the pipe buffer [4096 bytes] It works for linux 2.6 and windows as 
these support larger pipe buffers.  
See:http://forums.slimdevices.com/showthread.php?t=15194 (post 8)

To support PCM streams RETRY_TIME needs to be reduced so that it sustains the 
data rate.  However this increases cpu on slow servers and is unnecessary for 
low bit rate (mp3) streaming into the player.  

Bug as placeholder for a more elegant alorithm which adapts to rate and buffer 
size. [I will look at it, but will need some feedback from the author of this 
code]
Comment 1 KDF 2005-07-22 10:51:31 UTC
vidur, this was your section of code, correct?
Comment 2 Adrian Smith 2005-07-23 15:53:28 UTC
Created attachment 665 [details]
Possible patch

Michael - could you try this?  

I did look at more complex solutions to try to optimise how long to wait for,
but didn't come up with anything which was always the same or lower cpu load! 
So this just attempts to quess when the problem occurs and switch to a lower
retry time.  I can' test so please try it out [test with --d_http to see which
retry time is used]

Vidur - this includes a possible typo fix for the line:
-			if ($chunkRef && length($chunkRef)) {
+			if ($chunkRef && length($$chunkRef)) {

I'm assuming this is what was meant?
Comment 3 Michael Herger 2005-07-24 00:26:46 UTC
I'm feeling silly - but I can't reproduce the problem any more... I updated to the latest nightly build and 
am streaming Real->FLAC with no problem (DRS3 from the AlienBBC->Others) to my SB2.

Normally the sound would become garbled after only a few seconds (<< 1 minute). But now... almost 10 
minutes and still as clear as it could be.

I'll try to get the garbling back today and test the patch asap.
Comment 4 Michael Herger 2005-07-24 01:04:43 UTC
It took only 35 minutes to get the expected result. I now applied your patch and am waiting to see 
whether it will work better. I'll give it a 2hrs test (before I have to leave).
Comment 5 Adrian Smith 2005-07-24 02:46:29 UTC
I think the garbling is dependant on the order in which the OS is scheduling 
different tasks and hence can be complex to reproduce.  Richard T noticed that 
simply turning on debugging stopped the garbling...

Can you confirm that the fast retry time is used on your OS [0.02 rather than 
0.05]
Comment 6 Michael Herger 2005-07-25 08:00:34 UTC
I'm a little late with the results of my test. But after applying the patch I 
listened to that stream for more than 2.5h before I had to leave. Seems to be 
working for _this_ case - I haven't tested whether it will introduce new 
problems.
Comment 7 Vidur Apparao 2005-07-25 10:19:53 UTC
kdf's patch maybe be a reasonable stopgap, but maybe the right fix is to compute
a retry time based on the bitrate of the stream?
Comment 8 Adrian Smith 2005-07-25 11:15:25 UTC
Vidur,

[assume you meant my patch!]

I tried something to compute the time more accurately, but to get it right 
requires computing an average of the previously transfered data as at least 
some transcoding apps [mplayer] can send large bursts to slimserver (which are 
all read as the retry time is only involked when nothing can be read).  Hence 
to deal with such cases requires computing the average rate over the previous 
period and basing the retry on this.  In the end I settled on a simple 
solution as it only appears a problem on OSs with a 4K pipe size [linux 2.4 
+ ?]

My ideal would be to use select to trigger reading as well as writing into a 
buffer with thresholds to take the reader off select if it fills up too much 
etc.  However this looks to be a more significant change...
Comment 9 Adrian Smith 2005-07-27 12:17:58 UTC
Michael - any more feedback?

Vidur - do we want to consider committing this? [+ is the typo fix correct?]
Comment 10 Michael Herger 2005-07-27 13:03:46 UTC
I'm sorry, can't give much more feedback than "it works for me"
Comment 11 Vidur Apparao 2005-07-27 13:06:22 UTC
Cc:ing Dean to get another pair of eyes on it.
Comment 12 Blackketter Dean 2005-07-27 13:21:43 UTC
I agree, the patch looks like a reasonable stopgap.  
Comment 13 Adrian Smith 2005-07-27 13:56:43 UTC
OK I'll commit to trunk - do we want to consider this for the 6.1 branch [as 
someone set the milestone to this (not me!)]
Comment 14 Vidur Apparao 2005-07-27 14:08:54 UTC
No, this hasn't yet come up as a big issue, so let's keep it out of the
maintenance release.
Comment 15 KDF 2005-08-11 13:39:46 UTC
original patch in trunk on change 3806.  retitling to specify the suggested
possible future fix. marking as enhancement since current problem is reported as
fixed with stopgap.
Comment 16 Adrian Smith 2008-07-22 10:38:12 UTC
Doesn't seem to have been a problem for 3 years - closing...
Comment 17 James Richardson 2008-12-15 13:06:53 UTC
This bug appears to have been fixed in the latest release!

If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.

Make sure to include the version number of the software you are seeing the error with.