New subject: [ANN] guitarix-0.04.4-1 release

24 May 2009

Hi Hermann,

Great, I tested it against jack1 and jack2, and it is working fine now :)
I looked at your additions. I have a couple of comments, nothing you should consider as
important or critical:

- guitarix.cpp, line 529-530:
I think any jack function will check if the client pointer passed as an argument is valid
so it is safe to call jack_get_buffer_size before checking the client pointer validity.
However, I have the habit of checking pointer validity before passing it to a 3rd party
function. You never know, when the 3rd party lib is closed-source and its API badly
documented,  you may get crashes. It's not the case here so you can keep it like
that.

- guitarix.cpp, line 570-579: I would create a local function for this block of code
because you call it in some other place as well. You will have only one place to maintain
and gain more control on the variation of gNumOutChans. Turn it into:

if (merke == 0)
{
    gtx_unregister_ports();
}

- german looking variable names: I have nothing against german, even learned it at school
:D but it's open-source, not open-german-source  ;)

- I would also formalize a bit more what is guitarix's code so someone can tell right
away which is your code and which is not. For example, start all your home made standalone
functions with gtx_ (for GuiTariX)
For home made classes, GtxSomeClass
For class members of home made classes: fGtxSomeMember
For class methods of home made classes: gtxSomeMethod()

This will become much more readable.

You don't have to though, and cleaning up is a bit of a tedious job ... but if it is
to grow as you add more functionality, keep this in mind :)

Thanks for your work, I heard that you had to spend quite some time last night to make my
patch fit in for jack1 ;)

Tschuess :)
J.

--- On Sun, 5/24/09, hermann meyer &lt;brummer-(a)web.de&gt; wrote:

...
  From: hermann meyer &lt;brummer-(a)web.de&gt;
 Subject: Re: [LAD]  [ANN] guitarix-0.04.4-1 release
 To: warjamy(a)yahoo.com
 Cc: linux-audio-dev(a)lists.linuxaudio.org
 Date: Sunday, May 24, 2009, 2:17 AM
 Am Samstag, den 23.05.2009, 05:04
 -0700 schrieb warjamy(a)yahoo.com:
  discussion started at LAD about changing the jack
 server latency from the app called guitarix (like ardour
 does from its menu JACK -> Latency). Proposed guitarix
 patch (by me) works nice against jack2 but not against
 jack1. See below.

 Hermann,

 I quickly ran guitarix in debug mode through gdb. You  get a crash at
dsp_audio.cpp, line 469. Looks like memory at
 output[2] and output[3] cannot be accessed. I tried to
 understand what you attempt to do but this function
 (mydsp::compute) is way to much for me to understand. It
 looks like some sort of auto-code ...

 Anyway, output[2] and output[3] are passed in from  process() (process callback
function). I believe these
 arrays should correspond to buffers that you receive from
 jack audio ports 2 and 3. However, at the time we try to
 change the latency, they are already unregistered because of
 DSP.setNumOutputs().

 So it looks to me that resetting the jack buffer size  on the fly from guitarix
badly interacts with how jack1
 handled your port deregistration requests. I don't know why
 it works with jack2. Running guitarix through gdb against
 jack2, I could see that output[2] and output[3] were still
 valid memory locations after changing the jack buffer size
 on the fly. 

 This is a bit unfortunate that we get different  behaviors when using jack1 or
jack2. I hope you'll sort it
 out, I don't have more time to spend on it. 

 J.
   Morning James

 So I got it working. Your Code is submit to SVN with some
 little
 additions. Thanks James, that is the "first direct code
 contribution"
 the guitarix projekt recive, I hope more will follow. :D