On Tue, Jun 30, 2009 at 08:09:45AM +0200, Takashi Iwai wrote:

At Tue, 30 Jun 2009 02:11:01 +0400, Paul Fertser wrote:

...

I've tried to implement an external shared object to be able to execute arbitrary functions on PCM device opening and closing. After quite some source code reading i had an almost working but segfaulting example. This segfault is because snd_dlclose is called right after calling a hook install function and therefore i can't snd_pcm_hook_add functions from the same shared object. To me it looks like a bug but probably i just don't see how this functionality is intended to be used. Takashi, git-blame showed that it's you who stuffed this dlclose call that bothers me ;)

Well, right now, a plugin is designed to be bound with a single PCM instance. That's why dlcolse is called in snd_pcm_close().

It looks like we're talking about different things. You seem to be talking about external PCM plugins, i'm talking about external functions for the "hook" PCM plugin. The code i question is in pcm/pcm_hooks.c in function snd_pcm_hook_add_conf:

if (err >= 0) { if (args && snd_config_get_string(args, &str) >= 0) { err = snd_config_search_definition(root, "hook_args", str, &args); if (err < 0) SNDERR("unknown hook_args %s", str); else err = install_func(pcm, args); snd_config_delete(args); } else err = install_func(pcm, args); snd_dlclose(h); }

Can you give your code to grasp more implementation images?

Rough sketch attached.

...

We're working on audio subsystem for OpenMoko Freerunner. It's an idea of Joerg Reisenweber to have virtual alsa devices for every reasonable use-case (like dedicated device for stereout, dedicated device for recording from gsm etc), for that we need a hook on every device opening and closing mostly to set up routing inside WM8753 codec. ctl_elems is not enough because we need a more complex locking scheme (with priorities, some kind of callbacks to the apps using particular devices etc) and other flexibility.

To hook off snd_pcm_open and snd_pcm_close, you could use LD_PRELOAD. But, a plugin sounds more interesting.

Much more, i'd say.

On Tue, Jun 30, 2009 at 08:33:15AM +0200, Takashi Iwai wrote:

...

It looks like we're talking about different things. You seem to be talking about external PCM plugins, i'm talking about external functions for the "hook" PCM plugin. The code i question is in pcm/pcm_hooks.c in function snd_pcm_hook_add_conf:

    if (err >= 0) {
            if (args && snd_config_get_string(args, &str) >= 0) {
                    err = snd_config_search_definition(root,
            "hook_args", str, &args);
                    if (err < 0)
                            SNDERR("unknown hook_args %s", str);
                    else
                            err = install_func(pcm, args);
                    snd_config_delete(args);
            } else
                    err = install_func(pcm, args);
            snd_dlclose(h);
    }

Ah, I understand now. Yes, this looks like a bug. Meanwhile, it makes another bug if we just remove it. There would be no dlclose(), then. It'd need resource tracking.

Exactly the reason i haven't tried to make a proper patch ;)

A lack of dlclose is no serious bug, though.

I hope you know better than me ;)

So... just removing snd_dlclose() there fixes your problem? If yes, I'll commit the fix.

TBH, i've not tried but i'm pretty sure it will fix the problem. I tried with dlopen workaround (to increase the refcounter) in install function and i tried with LD_PRELOAD'ing the library before starting an alsa app; both workarounds worked.

On Tue, Jun 30, 2009 at 11:21:39AM +0200, Takashi Iwai wrote:

Paul Fertser wrote:

...

TBH, i've not tried but i'm pretty sure it will fix the problem. I tried with dlopen workaround (to increase the refcounter) in install function and i tried with LD_PRELOAD'ing the library before starting an alsa app; both workarounds worked.

OK, I fixed this on GIT now.

Wow! Thanks a lot for this fast and proper fix! I compile-tested it, will try running later, but it should be ok. :)

While reading the diff i saw you introduced several unused variables, patch attached.