[pulseaudio-discuss] [PATCH v2 2/3] alsa-mixer: autodetect the HDMI jack PCM device

[Tanu Kaskinen]

On Tue, 2018-02-13 at 08:28 +0530, Arun Raghavan wrote:
> On Sun, 8 Oct 2017, at 10:18 PM, Tanu Kaskinen wrote:
> > This removes the need to hardcode the PCM device index in the HDMI jack
> > names. The hardcoded values don't work with the Intel HDMI LPE driver.
> > 
> > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=100488
> > ---
> >  src/modules/alsa/alsa-mixer.c                      | 53 ++++++++++++++++++++--
> >  src/modules/alsa/alsa-mixer.h                      |  4 +-
> >  src/modules/alsa/alsa-sink.c                       |  2 +-
> >  src/modules/alsa/alsa-source.c                     |  2 +-
> >  .../alsa/mixer/paths/analog-output.conf.common     |  4 ++
> >  src/modules/alsa/mixer/paths/hdmi-output-0.conf    |  3 +-
> >  src/modules/alsa/mixer/paths/hdmi-output-1.conf    |  3 +-
> >  src/modules/alsa/mixer/paths/hdmi-output-2.conf    |  3 +-
> >  src/modules/alsa/mixer/paths/hdmi-output-3.conf    |  3 +-
> >  src/modules/alsa/mixer/paths/hdmi-output-4.conf    |  3 +-
> >  src/modules/alsa/mixer/paths/hdmi-output-5.conf    |  3 +-
> >  src/modules/alsa/mixer/paths/hdmi-output-6.conf    |  3 +-
> >  src/modules/alsa/mixer/paths/hdmi-output-7.conf    |  3 +-
> >  13 files changed, 73 insertions(+), 16 deletions(-)
> > 
> > diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-
> > mixer.c
> > index 02ab4a611..eaee7ea0a 100644
> > --- a/src/modules/alsa/alsa-mixer.c
> > +++ b/src/modules/alsa/alsa-mixer.c
> > @@ -1812,12 +1812,31 @@ static int element_probe(pa_alsa_element *e, 
> > snd_mixer_t *m) {
> >      return 0;
> >  }
> >  
> > -static int jack_probe(pa_alsa_jack *j, snd_mixer_t *m) {
> > +static int jack_probe(pa_alsa_jack *j, pa_alsa_mapping *mapping, 
> > snd_mixer_t *m) {
> >      bool has_control;
> >  
> >      pa_assert(j);
> >      pa_assert(j->path);
> >  
> > +    if (j->append_pcm_to_name) {
> > +        char *new_name;
> > +
> > +        if (!mapping) {
> > +            /* This could also be an assertion, because this should 
> > never
> > +             * happen. At the time of writing, mapping can only be NULL 
> > when
> > +             * module-alsa-sink/source synthesizes a path, and those
> > +             * synthesized paths never have any jacks, so jack_probe() 
> > should
> > +             * never be called with a NULL mapping. */
> > +            pa_log("Jack %s: append_pcm_to_name is set, but mapping is 
> > NULL. Can't use this jack.", j->name);
> > +            return -1;
> > +        }
> > +
> > +        new_name = pa_sprintf_malloc("%s,pcm=%i Jack", j->name, 
> > mapping->hw_device_index);
> > +        pa_xfree(j->alsa_name);
> > +        j->alsa_name = new_name;
> > +        j->append_pcm_to_name = false;
> 
> Series looks good to me. Just one question -- why do we reset
> append_pcm_to_name here? Presumably because jack_probe() is called
> multiple times on the jack? Or are you being defensive here?

I think it just felt right that once the pcm= string has been appended,
the flag should be cleared, because the appending is no longer needed.
It can be thought as a defensive measure, but probably the original
motivation had more to do with a desire to have good feng shui or
something.

Thanks for the review! I'll push this to master.

-- 
Tanu

https://liberapay.com/tanuk
https://www.patreon.com/tanuk