[asterisk-bugs] [Asterisk 0017659]: [patch] Using app_jack with JACK_HOOK will result in noise rather than the channel sound.

[Asterisk Bug Tracker]

The following issue has been UPDATED. 
====================================================================== 
https://issues.asterisk.org/view.php?id=17659 
====================================================================== 
Reported By:                mrwho
Assigned To:                
====================================================================== 
Project:                    Asterisk
Issue ID:                   17659
Category:                   Applications/app_jack
Reproducibility:            sometimes
Severity:                   major
Priority:                   normal
Status:                     new
Asterisk Version:           SVN 
JIRA:                        
Regression:                 No 
Reviewboard Link:            
SVN Branch (only for SVN checkouts, not tarball releases):  trunk 
SVN Revision (number only!): 274530 
Request Review:              
====================================================================== 
Date Submitted:             2010-07-16 05:50 CDT
Last Modified:              2010-07-16 10:53 CDT
====================================================================== 
Summary:                    [patch] Using app_jack with JACK_HOOK will result in
noise rather than the channel sound.
Description: 
  Using JACK_HOOK to manipulate the sound of a channel may result in noise
rather than proper sound. It was quite hard to squash this bug, but it's
all related to the way the ringbuffers behave.

  When a channel is set up app_jack will start queuing frames from jackd
to the ringbuffer to be sent to asterisk, problem being that at that point
the channel may be not yet ready - say that you're doing a Dial next - and
the buffer fills up.

  Once the buffer is filled the next frames coming in may be cut. If a
float - made of 4 bytes - could be only partially written, that would end
up in a consequent succession of "wrong" samples, all off by one, in other
words: noise.

  A bit of ASCII art to make the point, say that we have three samples
(A,B and C) coming from jack looking like:

<pre>
  ------A------ ------B------ ------C------
  |A1|A2|A3|A4| |B1|B2|B3|B4| |C1|C2|C3|C4|
</pre>

  Now, on the ringbuffer we have space only for 7 bytes, what will be
written in the buffer will look like:

<pre>
  ------A------ ----B-----
  |A1|A2|A3|A4| |B1|B2|B3|
</pre>

  When the next samples (D,E) come in - and say we have the space to write
them completely - our ringbuffer will look like:

<pre>
  ------A------ ------B------ ------D------ ----E-----
  |A1|A2|A3|A4| |B1|B2|B3|D1| |D2|D3|D4|E1| |E2|E3|E4|
</pre>

  When those samples will be sent to Asterisk they will only be noise,
since they're all off by one.

  The same issue may happen if - for any reason - the buffer is filled by
any other reason. This will happen often expecially if  - to have very low
latency - you're using very small buffers in app_jack and short periods in
jackd.

  We have three possible solutions to this issue:
<ol>
<li>We free, by reading, enough bytes from the ringbuffer and then write
the
      new samples. So we would drop the older frames, we just have to make
sure
      to drop entire frames and not only parts of them - else we are
simply
      reintroducing the same problem;</li>
<li>When we don't have space we will drop the whole buffer and start
writing
      from the beginning again. With small buffers this is not really an
issue
      and we avoid running into a case where we simply keep writing at the
top
      of the buffer dropping each time a few frames from the bottom of
it;</li>
<li>We use (1) for a set amount of consecutive times, after which we do
(2).
      In this way we have a chance to drop very few frames in case of a 
      momentary buffer underrun.<li>
</ol>

  The patch I'm attaching at the moment for testing uses (2), it's very
well suited for small buffers. I'm working on a patch to allow (1) (2) and
(3) to
be chosen as parameters to the JACK_HOOK function.  There is (1) in the
patch
as well, but it has been commented out, you may want to test drive it to
see
how it works on your environment.


====================================================================== 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2010-07-16 10:53 lmadsen        Description Updated                          
======================================================================