[LAD] [ANN] guitarix-0.04.4-1 release

[James Warden]

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 <brummer- at web.de> wrote:

> From: hermann meyer <brummer- at web.de>
> Subject: Re: [LAD]  [ANN] guitarix-0.04.4-1 release
> To: warjamy at yahoo.com
> Cc: linux-audio-dev at lists.linuxaudio.org
> Date: Sunday, May 24, 2009, 2:17 AM
> Am Samstag, den 23.05.2009, 05:04
> -0700 schrieb warjamy at 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
>  
> 
>