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

[Arun Raghavan]

On 26 April 2016 at 11:59, Tanu Kaskinen <tanuk at iki.fi> wrote:
> On Tue, 2016-04-26 at 08:52 +0530, Arun Raghavan wrote:
>> 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.
>
> We've had this problem before. According to this[1] blog post, the
> issue was fixed earlier by linking json-glib with -Bsymbolic. The
> option forces json-glib's internal json_object_get_type() calls to
> always use json-glib's own implementation, but I don't think it helps
> applications that use json-glib.
>
> We have many other json_object_get_type() calls in the code too. I
> don't know why those don't cause as bad results...

I'm guessing we don't hit those code paths in any client code (yet).

> Do you remember what motivated you to write the patch that caused this
> breakage? It looks like something that we could somewhat safely just
> revert for 9.0. After the 9.0 release, however, I think we really

I saw this while checking how rate validation works when updating
PA_RATE_MAX. So it wasn't to fix a specific manifestation of a bug,
and we can revert.

> should either get rid of the json-c dependency, or bundle it in
> pulseaudio and link to it statically.

I've started implementing our own parser, since I think that's cleaner
than copying over existing library code and having to either maintain
that or try to follow upstream.

For now though, I'll just revert that commit later today unless
someone has any other ideas for dealing with the problem.

-- Arun