Re: [FFADO-devel] file saving of ffado-mixer controls setting.

[Jonathan W. <jw...@ju...>]

Hi Phil

On Sun, Nov 10, 2013 at 05:16:29PM +0100, Philippe Carriere wrote:
> enclosed are patches for the purpose, accounting for the
> recommendations.
> I included something for RME: of course, I cannot pretend anything about
> except it gives you a basis for debugging on your own device :-)

I have finally had a chance to take a look at them.

> First and last patches have nothing to do with the topic ... The former
> is to avoid an usual error when starting ffado-mixer after complete
> recompilation: only non-english install are involved; typically, in
> french, directory is "Répertoire" and the "é" requires the support of
> utf-8. The latter is an attempt to refresh the list of authors: there
> are probably many omissions ...

I can't see any problems with that first patch.  You could commit that when
ready.

Regarding the bulk of the patch series, nothing in the patches jumps out at
me as being problematic.  I haven't had a chance to test the code with the
RME and probably won't have an opportunity until after a projection is
complete this coming weekend.  Within the confines of the mixer application
though it seems to be a workable approach.

I guess the primary disadvantage is that it requires a considerable amount
of code in each mixer module to handle the saving and restoration.  It would
be ideal if this could be handled in a way which was mostly transparent to
the mixer modules, but it seems that this is not something which would come
easiler.  I have looked for and failed to find a generic dbus program which
can read and restore a tree of settings under dbus - if something like this
existed we could use it with very little addition to ffado-mixer itself.  It
could just walk the tree created by FFADO and read or write the settings as
appropriate.  The fact that nothing like this seems to exist probably means
that traversing a dbus tree isn't all that trivial.

So with that said the approach you've taken is probably the next best thing
available to us and as such it can probably go in.  My only comment in
relation to this is to ponder whether the save/restore code for each mixer
module should be placed in a separate file in the interests of keeping the
mixer modules themselves clear of save/restore code.  I'm not sure how
feasible that would be, and in some ways would simply move the "clutter"
into the module directory as each mixer would now comprise two files.

In the long run I wonder whether we could arrange for FFADO to construct (at
run time) something to do the save/restore.  The structure of the mixer for
a given device is set by a series of calls to buildMixer().  Would it be
possible to leverage this to create something which could be used to walk
the mixer tree and save/restore as necessary.  Ffado-mixer would still need
the menu hooks to instigate save/restore, but the bulk of the work would be
handled by code which was essentially auto-generated at runtime.  If this
could be made to work it would remove the need to add save/restore code to
every ffado-mixer module.  However, I expect putting something like this
together will be a considerable amount of work - assuming that it's even
possible.  For that reason I don't think that the possibility of something
like this emerging in future should prevent this present work going in.

So on balance, given that this patch series provides useful functionality 
which can work now, I can't see any reason not to polish it (if deemed
necessary) and commit it.

Apologies again for the delay in reviewing this patch series.

Regards
  jonathan