luis4god / darkice

Automatically exported from code.google.com/p/darkice
0 stars 0 forks source link

BufferedSink makes streams invalid #57

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
* What steps will reproduce the problem?

1. Create bandwith limitation by uploading big stream(s) via a DSL
2. Play stream(s), the stream will be invalid (truncated, missing frames, ...)

* What is the expected output? What do you see instead?

The streams should be valid (mpeg frame or ogg page should be valid) even 
bandwith is missing to upload correctly.

* What version of the product are you using? On what operating system?

Darkice trunk

According to our tests, when the upload bandwith is too low, the streams
received by streaming servers are invalid. We define 3/4 big streams and
send them via a DSL line to reproduce the problem.

We've inspected this morning svn sources and we wonder how does
BufferedSink protect integrity of mp3 frame or ogg pages ? Encoders
write frames or pages, TcpSocket manages only bytes and
BufferedSink::store moves pointers without knowning frame/pages bounds.

If this conclusion is the right one, we think about two solutions :

Solution 1: manage frames/pages in BufferedSink (difficult)

Solution 2: experiment a new Sink order to use BufferedSink to store PCM
sound. The BufferedSink->chunkSize (unused currently) is perfect to
manage sample length.

Current sink order: Encoder -> CastSink -> BufferedSink -> TcpSocket
New sink order: BufferedSink -> Encoder -> CastSink -> TcpSocket

We expect these required changes for this solution 2 :

* initialize BufferedSink with sample length as chunkSize and the
AudioEncoder as next Sink when Output is created by
Darkice::configIceCast2/Shoutcast
* attach BufferedSink directly with Connector
* improve Encoder::canWrite to manage its encoder status and the next
Sink status (returns true for the moment)

Cons: It would use more memory, because each Output will store few
seconds of PCM data (170kb/sec).
Pros: Darkice streams will be valid \o/

Here is a small patch which implements this idea :
https://github.com/tryphon/darkice/tree/buffer-before-encoder. It
changes the sink order as explained on our previous email. The two related 
commits are in the attached file.

We've performed tests with limited bandwith. The darkice streams remain
valid. There is not enough data for listeners but no invalid frame.

Original issue reported on code.google.com by alban.pe...@gmail.com on 31 Oct 2011 at 2:12

Attachments:

GoogleCodeExporter commented 9 years ago
applied in r503.

Original comment by rafael2k...@gmail.com on 16 Jan 2012 at 8:59