Re: [ucm] Adding list for caching multiple opened control devices

[Daniel Mack <zonque@gmail.com>]

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 :)


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@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>