[pulseaudio-discuss] [PATCH v4 1/2] client audio: Support memfd transport

[Arun Raghavan]

On Mon, 2016-04-25 at 23:27 +0530, Arun Raghavan wrote:
> On 25 April 2016 at 22:29, Ahmed S. Darwish <darwish.07 at gmail.com> wrote:
> > 
> > Hi!
> > 
> > Interesting discoveries ahead ..
> > 
> > On Mon, Apr 25, 2016 at 08:28:40PM +0530, Arun Raghavan wrote:
> > > 
> > > On 25 April 2016 at 18:20, Ahmed S. Darwish <darwish.07 at gmail.com> wrote:
> > > > 
> > > > 
> > > > Nonetheless, of course what you're proposing is the more correct
> > > > logic. So if this is the only problem, let's merge and I'll create
> > > > a follow-up patch this week that introduces the desired macros and
> > > > fixes up this version-check issue in the process.
> > > Not to worry, I actually ended up fixing it locally.
> > Thanks!
> > 
> > > 
> > > Still testing
> > > things, but I found an interesting breakage that I can't yet explain.
> > > I merged your patches and built the code, and the did something like:
> > > 
> > >   $> PULSE_LOG=4 LD_LIBRARY_PATH=$PWD/build/src/.libs gst-launch-1.0
> > > playbin uri=file:///path/to/some/file
> > > 
> > > which works fine (and I see logs about memfd being selected).
> > > 
> > Just installed a shiny gstreamer-1.8 here. Simple commands like:
> > 
> >     $ gst-launch-1.0 audiotestsrc ! pulsesink
> > 
> > are working nicely here too ..
> > 
> > > 
> > > But,
> > > 
> > >   $> PULSE_LOG=4 LD_LIBRARY_PATH=$PWD/build/src/.libs rhythmbox (or totem)
> > > 
> > > seems to fail with the following error:
> > > 
> > >   Format info property 'format.sample_format' type is not string.
> > > 
> > > Which is weird because the two lines should be doing the same thing.
> > > 
> > Yup, I can replicate the above, with and without the memfd patch
> > series .. Seems we've got a new v9.0 release blocker :-(
> > 
> > Here are some quick debugging results:
> > 
> > - Totem with PulseAudio v8.0 is working nicely
> > 
> > - Totem with PulseAudio master, before merging any memfd patches,
> >   commit 0f48b7c82378c32194b625d1fd886eee7f1e5928 .. still has the
> >   bug
> > 
> > Here are more details on the bug, running the PA master branch
> > commit 0f48b7c82378c above:
> > 
> > $ GST_DEBUG=2 PULSE_LOG=4 totem brown.wav
> [...]
> 
> > 
> > Format info property 'format.sample_format' type is not string.
> [...]
> > 
> > gstreamer pulsesink is complaining several times of not being
> > able to "create probe stream" ..
> The line I've snipped out suggests a break in protocol.
> 
> > 
> > > 
> > > 
> > > I'll try to pin this down later tonight or tomorrow. Hopefully just a
> > > glitch in my setup.
> > > 
> > I'll submit a bug report now, and mark it as a blocker. I'm
> > currently working on another blocker [*], but this seems to have
> > a much higher priority (a user-facing program not working) ..
> > 
> > [*] https://bugs.freedesktop.org/show_bug.cgi?id=95021
> 
> Thanks for investigating! I'll try to take a look too.

Okay, so this is a fun one. The offending commit is:

  commit 12a202c510dcacbd2b85fcc1170484eb16fef491
  Author: Arun Raghavan <git at arunraghavan.net>
  Date:   Thu Dec 31 09:27:56 2015 +0530

      format: Make pa_format_info_valid() stricter for PCM
   
      We should do stricter validation when we can.

The check fails at:

  if (json_object_get_type(o) != json_type_string) {
      pa_log_debug("Format info property '%s' type is not string.", key);
      json_object_put(o);
      return -PA_ERR_INVALID;
  }

Rhythmbox and Totem are linked to the json-glib library which *also*
has a json_object_get_type(). That is the function which gets called,
and returns a junk type which makes this code fail.

I don't know what the right way to fix this sort of situation with
colliding symbols. The worst case, I guess, is to drop json-c
altogether and maybe just do our own lightweight parsing for these
strings. Would be nice to not have to do that right now, though, so I'm
all ears for suggestions.

Cheers,
Arun