[pulseaudio-discuss] [PATCH] pavucontrol: Show available status

[Tanu Kaskinen]

On Thu, 2012-07-19 at 16:34 +0200, David Henningsson wrote:
> If we know if a certain port is available/unavailable, we can print
> that out, as a help to the user (and as debugging for ourselves).
> A profile is also available/unavailable if all ports which have that
> profile is available/unavailable.
> ---
>  src/mainwindow.cc  |   44 ++++++++++++++++++++++++++++++++++++++------
>  src/pavucontrol.cc |   16 ++++++++++++++++
>  src/pavucontrol.h  |    1 +
>  3 files changed, 55 insertions(+), 6 deletions(-)
> 
> diff --git a/src/mainwindow.cc b/src/mainwindow.cc
> index dc84682..b843d03 100644
> --- a/src/mainwindow.cc
> +++ b/src/mainwindow.cc
> @@ -285,8 +285,22 @@ void MainWindow::updateCard(const pa_card_info &info) {
>      }
>  
>      w->profiles.clear();
> -    for (std::set<pa_card_profile_info>::iterator i = profile_priorities.begin(); i != profile_priorities.end(); ++i)
> -        w->profiles.push_back(std::pair<Glib::ustring,Glib::ustring>(i->name, i->description));
> +    for (std::set<pa_card_profile_info>::iterator i = profile_priorities.begin(); i != profile_priorities.end(); ++i) {
> +        gchar* desc = NULL;
> +        int sums[3] = {0, 0, 0};
> +#ifdef HAVE_PORT_AVAILABILITY
> +        for (pa_card_port_info** cp = info.ports; *cp; cp++)
> +            for (pa_card_profile_info** pp = (*cp)->profiles; *pp; pp++)

I'd prefer "port" instead of "cp" and "profile" instead of "pp" for
readability reasons. I guess you have different preferences?

> +                if ((*pp)->name == i->name && (*cp)->available < 3)
> +                    sums[(*cp)->available]++;
> +        if (sums[PA_PORT_AVAILABLE_NO] && !sums[PA_PORT_AVAILABLE_YES] && !sums[PA_PORT_AVAILABLE_UNKNOWN])
> +            desc = g_strdup_printf(_("%s (unplugged)"), i->description);
> +        else if (sums[PA_PORT_AVAILABLE_YES] && !sums[PA_PORT_AVAILABLE_NO] && !sums[PA_PORT_AVAILABLE_UNKNOWN])
> +            desc = g_strdup_printf(_("%s (plugged in)"), i->description);

I don't like this kind of cleverness, I mean the sums array. Having the
array may help with minimizing the amount of lines of code, but it
relies on the AVAILABLE constants to have certain values, which in my
opinion is bad style.

> diff --git a/src/pavucontrol.cc b/src/pavucontrol.cc
> index 72ec980..6a5ff8b 100644
> --- a/src/pavucontrol.cc
> +++ b/src/pavucontrol.cc
> @@ -426,6 +426,22 @@ void subscribe_cb(pa_context *c, pa_subscription_event_type_t t, uint32_t index,
>                      return;
>                  }
>                  pa_operation_unref(o);
> +
> +                /* Because the port availability might have changed, and we don't send
> +                   update events for that, we must update sources and sinks belonging. 
> +                   to that card. Do it for all sources and sinks for simplicity. */

Trailing whitespace after "belonging." Please do
"mv .git/hooks/pre-commit.sample .git/hooks/pre-commit" to prevent
trailing whitespace from slipping in the commits. Also, the period after
"belonging" shouldn't be there.

> +                if (!(o = pa_context_get_sink_info_list(c, sink_cb, w))) {
> +                    show_error(_("pa_context_get_sink_info_list() failed"));
> +                    return;
> +                }
> +                pa_operation_unref(o);
> +
> +                if (!(o = pa_context_get_source_info_list(c, source_cb, w))) {
> +                    show_error(_("pa_context_get_source_info_list() failed"));
> +                    return;
> +                }
> +                pa_operation_unref(o);

Do we really want to make two more round-trips here? The port
availability information is already included in the card info.

-- 
Tanu