[alsa-devel] [ucm] Adding list for caching multiple opened control devices

[Takashi Iwai]

At Mon, 24 Jun 2013 19:09:33 +0200,
Daniel Mack wrote:
> 
> On 23.06.2013 00:10, sajeesh sidharthan wrote:
> > Hello.
> >
> > I would like to contribute to alsa development. As a first step, I
> > have made changes for caching control devices in ucm.
> 
> Thank you for doing this.
> 
> > Please review the following changes and let me know if I can check in
> > the changes and how to check in?
> 
> First of all, please do not hijack an email thread by clicking on the
> 'reply' button and changing the subject line. An email contains internal
> references to other mails in the same thread that most email clients
> won't allow you to see or edit. Your email was hence hidden inside
> another thread, and most likely it was overlooked by the UCM maintainer.
> 
> Second, you should always Cc: the relevant maintainers in your emails.
> In this case, Liam and Takashi. I copied them now.
> 
> And third, please use 'git commit', 'git format-patch' and 'git
> send-email', which will format everything correctly for you. And note
> that you need to add a 'Signed-off-by:' line with your name and email
> address, as done in other patches.
> 
> I can't really comment on the actual patch. Let's hope Liam or Takashi
> can :)

Well, for that, I need to understand the exact goal of the patch;
i.e. what it fixes or what it provides for which purpose.  The patch
description is as equally important as the code change in the patch
itself.

So, please give a bit more detailed (but concise) patch description.


thanks,

Takashi

> 
> 
> Daniel
> 
> 
> > ----------------------
> > 
> > diff --git a/src/ucm/main.c b/src/ucm/main.c
> > index 030c9b1..3219778 100644
> > --- a/src/ucm/main.c
> > +++ b/src/ucm/main.c
> > @@ -131,31 +131,33 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr,
> >              const char *ctl_dev)
> >  {
> >      int err;
> > +    struct list_head *pos;
> > +    struct snd_ctl_list *snd_ctl;
> >  
> > -    /* FIXME: add a list of ctl devices to uc_mgr structure and
> > -           cache accesses for multiple opened ctl devices */
> > -    if (uc_mgr->ctl_dev != NULL && strcmp(ctl_dev, uc_mgr->ctl_dev) == 0) {
> > -        *ctl = uc_mgr->ctl;
> > -        return 0;
> > -    }
> > -    if (uc_mgr->ctl_dev) {
> > -        free(uc_mgr->ctl_dev);
> > -        uc_mgr->ctl_dev = NULL;
> > -        snd_ctl_close(uc_mgr->ctl);
> > -    
> > +    list_for_each(pos, &uc_mgr->ctl_list) {
> > +        snd_ctl = list_entry(pos, struct snd_ctl_list, list);
> > +        if(!snd_ctl)
> > +            break;
> > +        if(!strcmp(snd_ctl->ctl_dev, ctl_dev)) {
> > +            *ctl = snd_ctl->ctl;
> > +            return 0;
> > +        }
> >      }
> >      err = snd_ctl_open(ctl, ctl_dev, 0);
> >      if (err < 0)
> >          return err;
> > -    uc_mgr->ctl_dev = strdup(ctl_dev);
> > -    if (uc_mgr->ctl_dev == NULL) {
> > -        snd_ctl_close(*ctl);
> > -        return -ENOMEM;
> > -    }
> > -    uc_mgr->ctl = *ctl;
> > +
> > +    if (*ctl) {
> > +        snd_ctl = malloc(sizeof(struct snd_ctl_list));
> > +        snd_ctl->ctl_dev = strdup(ctl_dev);
> > +        snd_ctl->ctl = *ctl;
> > +        list_add_tail(&snd_ctl->list, &uc_mgr->ctl_list);
> > +    }
> > +
> >      return 0;
> >  }
> >  
> > +
> >  static int execute_cset(snd_ctl_t *ctl, char *cset)
> >  {
> >      char *pos;
> > @@ -622,6 +624,7 @@ int snd_use_case_mgr_open(snd_use_case_mgr_t **mgr,
> >      INIT_LIST_HEAD(&uc_mgr->value_list);
> >      INIT_LIST_HEAD(&uc_mgr->active_modifiers);
> >      INIT_LIST_HEAD(&uc_mgr->active_devices);
> > +    INIT_LIST_HEAD(&uc_mgr->ctl_list);
> >      pthread_mutex_init(&uc_mgr->mutex, NULL);
> >  
> >      uc_mgr->card_name = strdup(card_name);
> > diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h
> > index 13e82da..86a51c7 100644
> > --- a/src/ucm/ucm_local.h
> > +++ b/src/ucm/ucm_local.h
> > @@ -82,6 +82,12 @@ struct dev_list {
> >  };
> >  
> >  
> > +struct snd_ctl_list {
> > +    snd_ctl_t *ctl;
> > +    char *ctl_dev;
> > +    struct list_head list;
> > +};
> > +
> >  /*
> >   * Describes a Use Case Modifier and it's enable and disable sequences.
> >   * A use case verb can have N modifiers.
> > @@ -183,8 +189,8 @@ struct snd_use_case_mgr {
> >      pthread_mutex_t mutex;
> >  
> >      /* change to list of ctl handles */
> > -    snd_ctl_t *ctl;
> > -    char *ctl_dev;
> > +    
> > +    struct list_head ctl_list;
> >  };
> >  
> >  #define uc_error SNDERR
> > diff --git a/src/ucm/utils.c b/src/ucm/utils.c
> > index 2def0b8..f18a2ac 100644
> > --- a/src/ucm/utils.c
> > +++ b/src/ucm/utils.c
> > @@ -99,6 +99,21 @@ void uc_mgr_free_value(struct list_head *base)
> >      }
> >  }
> >  
> > +
> > +void uc_mgr_free_ctl(struct list_head *base)
> > +{
> > +    struct list_head *pos, *npos;
> > +    struct snd_ctl_list *ctl_list;
> > +    
> > +    list_for_each_safe(pos, npos, base) {
> > +        ctl_list = list_entry(pos, struct snd_ctl_list, list);
> > +        free(ctl_list->ctl_dev);
> > +        snd_ctl_close(ctl_list->ctl);
> > +        list_del(&ctl_list->list);
> > +        free(ctl_list);
> > +    }
> > +}
> > +
> >  void uc_mgr_free_dev_list(struct list_head *base)
> >  {
> >      struct list_head *pos, *npos;
> > @@ -220,12 +235,7 @@ void uc_mgr_free_verb(snd_use_case_mgr_t *uc_mgr)
> >      uc_mgr->active_verb = NULL;
> >      INIT_LIST_HEAD(&uc_mgr->active_devices);
> >      INIT_LIST_HEAD(&uc_mgr->active_modifiers);
> > -    if (uc_mgr->ctl != NULL) {
> > -        snd_ctl_close(uc_mgr->ctl);
> > -        uc_mgr->ctl = NULL;
> > -    }
> > -    free(uc_mgr->ctl_dev);
> > -    uc_mgr->ctl_dev = NULL;
> > +    uc_mgr_free_ctl(&uc_mgr->ctl_list);
> >  }
> >  
> >  void uc_mgr_free(snd_use_case_mgr_t *uc_mgr)
> > 
> > 
> > 
> > 
> > ---------------------
> > 
> > Sajeesh
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel at alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> > 
>