[asterisk-bugs] [JIRA] (ASTERISK-16380) [patch] Using app_jack with JACK_HOOK will result in noise rather than the channel sound.

[Sean Bright (JIRA)]

     [ https://issues.asterisk.org/jira/browse/ASTERISK-16380?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sean Bright updated ASTERISK-16380:
-----------------------------------

    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:

{noformat}
------A------ ------B------ ------C------
|A1|A2|A3|A4| |B1|B2|B3|B4| |C1|C2|C3|C4|
{noformat}

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

{noformat}
------A------ ----B-----
|A1|A2|A3|A4| |B1|B2|B3|
{noformat}

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

{noformat}
------A------ ------B------ ------D------ ----E-----
|A1|A2|A3|A4| |B1|B2|B3|D1| |D2|D3|D4|E1| |E2|E3|E4|
{noformat}

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:

# 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;
# 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;
# 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.

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.

I started my experimentinig with app_jack and used for a while the patch attached to issue 0016023. However that patch has a flaw in the way it handles the AST_AUDIOHOOK_DIRECTION_READ, the fact that it does itself is wrong.

My understanding - and my usage so far - of app_jack is that it manipulates audio when it's being written on the channel, the calls to the functions that fill the jack output ringbuffer and that read from the input ringbuffer are correct in the original version of app_jack in they way they are.

Separating the two calls in two different directions makes it so that data written to the channel is not actually modified by the passage to jackd.

One last thing: the patch adds an option for app_jack to set the ringbuffer size from the dialplan. The option is "r(buffersize)".

  was:
  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.



****** ADDITIONAL INFORMATION ******

  I started my experimentinig with app_jack and used for a while the patch attached to issue 0016023. However that patch has a flaw in the way it handles the AST_AUDIOHOOK_DIRECTION_READ, the fact that it does itself is wrong.
  My understanding - and my usage so far - of app_jack is that it manipulates audio when it's being written on the channel, the calls to the functions that fill the jack output ringbuffer and that read from the input ringbuffer are correct in the original version of app_jack in they way they are.

  Separating the two calls in two different directions makes it so that data written to the channel is not actually modified by the passage to jackd.

  One last thing: the patch adds an option for app_jack to set the ringbuffer size from the dialplan. The option is "r(buffersize)".





> [patch] Using app_jack with JACK_HOOK will result in noise rather than the channel sound.
> -----------------------------------------------------------------------------------------
>
>                 Key: ASTERISK-16380
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-16380
>             Project: Asterisk
>          Issue Type: Bug
>          Components: Applications/app_jack
>            Reporter: Fabio Torchetti
>         Attachments: app-jack.patch, app-jack-rev2.patch, asterisk_debug_version0, asterisk_jack_output_fix_version0.patch
>
>
> 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:
> {noformat}
> ------A------ ------B------ ------C------
> |A1|A2|A3|A4| |B1|B2|B3|B4| |C1|C2|C3|C4|
> {noformat}
>   Now, on the ringbuffer we have space only for 7 bytes, what will be written in the buffer will look like:
> {noformat}
> ------A------ ----B-----
> |A1|A2|A3|A4| |B1|B2|B3|
> {noformat}
> When the next samples (D,E) come in - and say we have the space to write them completely - our ringbuffer will look like:
> {noformat}
> ------A------ ------B------ ------D------ ----E-----
> |A1|A2|A3|A4| |B1|B2|B3|D1| |D2|D3|D4|E1| |E2|E3|E4|
> {noformat}
> 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:
> # 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;
> # 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;
> # 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.
> 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.
> I started my experimentinig with app_jack and used for a while the patch attached to issue 0016023. However that patch has a flaw in the way it handles the AST_AUDIOHOOK_DIRECTION_READ, the fact that it does itself is wrong.
> My understanding - and my usage so far - of app_jack is that it manipulates audio when it's being written on the channel, the calls to the functions that fill the jack output ringbuffer and that read from the input ringbuffer are correct in the original version of app_jack in they way they are.
> Separating the two calls in two different directions makes it so that data written to the channel is not actually modified by the passage to jackd.
> One last thing: the patch adds an option for app_jack to set the ringbuffer size from the dialplan. The option is "r(buffersize)".



--
This message was sent by Atlassian JIRA
(v6.2#6252)