Bug in BufferedSink.cpp

[GoogleCodeExporter]

What steps will reproduce the problem?
1. external IceCast server,
2. and buffer overrun due to a temporary slowdown in network traffic.

After the network has recovered and is working at full bandwith again, 
streaming should continue without glitches, but instead it continues to have 
glitches until darkice is manually restarted.

I found the bug responsible for this problem in BufferedSink.cpp (Darkice 1.2). 
Suppose outp > inp and bufferEnd - outp - 1 < chunkSize, then the function 
cannot write anything to the underlying sink until store has been called so 
many times that the buffer experiences an overrun and only then a large lump of 
data is managed to be sent. This results in a loop of continuous overruns.

One solution would be to add the following line somewhere to the function.
if( bufferEnd - outp - 1 < chunkSize) outp = buffer;
This worked for me, but I do not know the code deeply enough to be sure this 
solution doesn't have any unwanted side effects.

Original issue reported on code.google.com by kulon...@gmail.com on 11 Jan 2014 at 3:33

[GoogleCodeExporter]

Ok, now I found out why the previous situation can ever happen.
In BufferedSink.cpp, where trying to write the outp -> bufferEnd, the size 
calculation is incorrect. 

Instead of
size    = bufferEnd - outp - 1;
it should be
size    = bufferEnd - outp;

I think this will fix most of the reported buffer issues.

Original comment by kulon...@gmail.com on 12 Jan 2014 at 10:55

[GoogleCodeExporter]

To be precise, nine of the other reported issues have definitely the same root 
cause:
11, 32, 37, 54, 58, 59, 71, 91, 92,
and possibly even:
34, 55, 89.

Removing those 4 excessive characters from BufferedSink.cpp asap would benefit 
a large group of users, it seems.

The reason why this bug doesn't actualize in all configurations is that, if 
streaming server is run on localhost, network is fast enough for bufferedSink 
to never use the ring buffer.

Original comment by kulon...@gmail.com on 13 Jan 2014 at 9:40

[GoogleCodeExporter]

Kulon, I was hopeful your "size = bufferEnd - outp" change would solve my 
buffer issues and, for a time, it appeared it did. Then I noticed my Vorbis 
stream hiccuping again when my remote Icecast server experienced some network 
packet loss:

18-Feb-2014 08:21:49 Exception caught in BufferedSink :: write3

18-Feb-2014 08:21:49 MultiThreadedConnector :: sinkThread reconnecting  0
18-Feb-2014 08:21:49 couldn't write full vorbis data to underlying sink 278
18-Feb-2014 08:21:53 ring buffer full, skipping data
18-Feb-2014 08:21:53 ring buffer full, skipping data
18-Feb-2014 08:21:54 HTTP/1.0 200

Should I combine this with your suggestion of...
  if( bufferEnd - outp - 1 < chunkSize) outp = buffer;
?

And if so, where should I add this line in BufferedSink.cpp?

Thanks!

Original comment by jmarktur...@gmail.com on 18 Feb 2014 at 3:21

[GoogleCodeExporter]

Jmarktur, I don't think this problem is related to the one I fixed. You 
shouldn't use the suggestion of..., because the removing of "- 1" is enough to 
fix the original issue. However, I tracked the cause of your problem.

Due to packet loss, there is a problem sending a packet to the server, which 
results in an exception rising to the BufferedSink from lower levels of code. 
The exception is then thrown up to MultiThreadedConnector, which in turn 
decides to reconnect the TCP connection. While reconnecting, ring buffer is 
filled and starts skipping data.

It can be fixed by adding a restriction to reconnect only after BufferedSink is 
filled. I'll create a new issue with a fix proposal.

Original comment by kulon...@gmail.com on 18 Feb 2014 at 9:45

[GoogleCodeExporter]

Hi Kulon,

Got any tips for implementing this? I'd like to give it a shot.

Thanks!

Original comment by jmarktur...@gmail.com on 20 Feb 2014 at 12:42

[GoogleCodeExporter]

Hi Jmarktur,

Darkice wasn't designed to deal with a remote server in the first place, it 
seems. I added issue 103 to explain some of the underlying issues.

After a more thorough examination of the code, I'm afraid there isn't an easy 
way out. Maybe we could unite our effort to fix this. I'll send you email to 
discuss more about the possible collaboration.

Original comment by kulon...@gmail.com on 20 Feb 2014 at 10:21

[GoogleCodeExporter]

Hi Kulon and Jmarktur,

Is there any progress on this ? it seems I have the same issue here

Original comment by oelja...@gmail.com on 24 May 2014 at 9:07