[pulseaudio-discuss] [PATCH] pavucontrol: add support for AAC pass-through

[Tanu Kaskinen]

On Tue, 2013-02-19 at 20:38 +0900, atsukada at users.sourceforge.net wrote:
> From: Akihiro Tsukada <atsukada at users.sourceforge.net>
> 
> ---
>  src/pavucontrol.glade | 19 ++++++++++++++++---
>  src/pavucontrol.h     |  1 +
>  src/sinkwidget.cc     | 10 ++++++++++
>  src/sinkwidget.h      |  2 +-
>  4 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/src/pavucontrol.glade b/src/pavucontrol.glade
> index 6defb3d..4b3dd01 100644
> --- a/src/pavucontrol.glade
> +++ b/src/pavucontrol.glade
> @@ -427,9 +427,6 @@
>                              <property name="n_rows">2</property>
>                              <property name="n_columns">3</property>
>                              <child>
> -                              <placeholder/>
> -                            </child>
> -                            <child>
>                                <object class="GtkCheckButton" id="encodingFormatPCM">
>                                  <property name="label" translatable="yes">PCM</property>
>                                  <property name="use_action_appearance">False</property>
> @@ -499,6 +496,22 @@
>                                  <property name="bottom_attach">2</property>
>                                </packing>
>                              </child>
> +                            <child>
> +                              <object class="GtkCheckButton" id="encodingFormatAAC">
> +                                <property name="label" translatable="yes">AAC</property>
> +                                <property name="use_action_appearance">False</property>
> +                                <property name="visible">True</property>
> +                                <property name="can_focus">True</property>
> +                                <property name="receives_default">False</property>
> +                                <property name="draw_indicator">True</property>
> +                              </object>
> +                              <packing>
> +                                <property name="left_attach">2</property>
> +                                <property name="right_attach">3</property>
> +                                <property name="top_attach">1</property>
> +                                <property name="bottom_attach">2</property>
> +                              </packing>
> +                            </child>
>                            </object>
>                            <packing>
>                              <property name="expand">False</property>
> diff --git a/src/pavucontrol.h b/src/pavucontrol.h
> index 65cb913..affaa89 100644
> --- a/src/pavucontrol.h
> +++ b/src/pavucontrol.h
> @@ -41,6 +41,7 @@
>  
>  #define HAVE_SOURCE_OUTPUT_VOLUMES PA_CHECK_VERSION(0,99,0)
>  #define HAVE_EXT_DEVICE_RESTORE_API PA_CHECK_VERSION(0,99,0)
> +#define HAVE_MPEG2_AAC_PASSTHROUGH PA_CHECK_VERSION(3,99,0)

This is not necessary. You can just check whether
PA_ENCODING_MPEG2_AAC_IEC61937 is defined. Or actually you can't,
because format.h doesn't define that, but I'll fix that. Fixed:
http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=773ca81dea1a18f93793ebfab4242d9a9ff73585

>  
>  enum SinkInputType {
>      SINK_INPUT_ALL,
> diff --git a/src/sinkwidget.cc b/src/sinkwidget.cc
> index 1726550..35b0aa7 100644
> --- a/src/sinkwidget.cc
> +++ b/src/sinkwidget.cc
> @@ -62,6 +62,16 @@ SinkWidget::SinkWidget(BaseObjectType* cobject, const Glib::RefPtr<Gtk::Builder>
>      encodings[i].encoding = PA_ENCODING_DTS_IEC61937;
>      x->get_widget("encodingFormatDTS", encodings[i].widget);
>      encodings[i].widget->signal_toggled().connect(sigc::mem_fun(*this, &SinkWidget::onEncodingsChange));
> +
> +    ++i;
> +    x->get_widget("encodingFormatAAC", encodings[i].widget);
> +#if HAVE_MPEG2_AAC_PASSTHROUGH
> +    encodings[i].encoding = PA_ENCODING_MPEG2_AAC_IEC61937;
> +    encodings[i].widget->signal_toggled().connect(sigc::mem_fun(*this, &SinkWidget::onEncodingsChange));
> +#else
> +    encodings[i].encoding = PA_ENCODING_INVALID;
> +    encodings[i].widget->set_sensitive(false);
> +#endif

I'm afraid this is not good enough. One question is what happens if
pavucontrol is compiled on a system with 4.0 headers, and run on a
system with 3.0 libpulse? In that case you pass libpulse an encoding
that it doesn't recognize. Well, it happens that libpulse doesn't
validate the encoding values before sending them to the server, so
there's no breakage. But it's still ugly to rely on the libpulse
implementation details. I'd remove the check altogether and just depend
on libpulse 4.0.

Another question is what does the server do if it doesn't support the
encoding? The answer is that saving the formats fails altogether, so the
check is clearly not good enough. You need to check whether the server
supports PA_ENCODING_MPEG2_AAC_IEC61937. How to do that? The check
should be based on the protocol version that the server support. That
can be queried with pa_context_get_server_protocol_version(). But
there's a problem: the protocol version was not incremented when the new
enumeration value was added.

Adding the new encoding wasn't as simple as it seemed... Incrementing
the protocol version is easy enough, but we also need to look at every
case where encodings are passed between a client and a server, and
decide the best way to handle situations where the receiving end doesn't
support the encoding that the sender would like to send. I'll start
working on this.

-- 
Tanu