[pulseaudio-discuss] [PATCH v2] module-coreaudio-device: get channel name as CFString and convert to plain C string.

[David Henningsson]

First, thanks for looking after OSX :-)

And then - could you try sending your patch with git send-email (or git 
format-patch and then attach the resulting file)? This looks weird, with 
half of the patch in an attachment and the header outside the attachment.

On 2015-04-19 08:57, Mihai Moldovan wrote:
>
> diff --git a/src/modules/macosx/module-coreaudio-device.c b/src/modules/macosx/module-coreaudio-device.c
> index cb62661..ad5c07b 100644
> --- a/src/modules/macosx/module-coreaudio-device.c
> +++ b/src/modules/macosx/module-coreaudio-device.c
> @@ -375,6 +375,39 @@ static int ca_sink_set_state(pa_sink *s, pa_sink_state_t state) {
>       return 0;
>   }
>
> +/* Caveat: this function frees the CFString if conversion succeeded. */

I'm not actually following the reasoning. Is the CFString leaked if the 
conversion fails, that can't be a good thing?

> +static int CFString_to_cstr_n(CFStringRef cfstr, char *buf, long n) {

Maybe return a bool here instead of int? Often an int result of 0 means 
success/no error, here's it's the opposite, which is confusing.

> +    int ret;
> +
> +    assert (buf);
> +
> +    ret = 0;
> +
> +    if (cfstr != NULL) {
> +        const char *tmp = CFStringGetCStringPtr(cfstr, kCFStringEncodingUTF8);
> +
> +        if (tmp == NULL) {
> +            if (CFStringGetCString(cfstr, buf, n, kCFStringEncodingUTF8))
> +                ret = 1;
> +        }
> +        else {
> +            strncpy(buf, tmp, n);
> +            buf[n - 1] = 0;
> +            ret = 1;
> +        }
> +    }
> +
> +    /*
> +     * A true value for ret implies cfstr != NULL, but let's still do the check
> +     * for safety reasons (i.e., should this code ever be re-organized...)
> +     */
> +    if (ret && cfstr != NULL) {
> +        CFRelease(cfstr);
> +    }

Can't you just move it a line above, inside the "if (cfstr != NULL)" 
part? Then you don't need to check for it again.

> +
> +    return ret;
> +}
> +
>   static int ca_device_create_sink(pa_module *m, AudioBuffer *buf, int channel_idx) {
>       OSStatus err;
>       UInt32 size;
> @@ -387,6 +420,7 @@ static int ca_device_create_sink(pa_module *m, AudioBuffer *buf, int channel_idx
>       char tmp[255];
>       pa_strbuf *strbuf;
>       AudioObjectPropertyAddress property_address;
> +    CFStringRef tmp_cfstr;
>
>       ca_sink = pa_xnew0(coreaudio_sink, 1);
>       ca_sink->map.channels = buf->mNumberChannels;
> @@ -401,7 +435,11 @@ static int ca_device_create_sink(pa_module *m, AudioBuffer *buf, int channel_idx
>           property_address.mScope = kAudioDevicePropertyScopeOutput;
>           property_address.mElement = channel_idx + i + 1;
>           size = sizeof(tmp);
> -        err = AudioObjectGetPropertyData(u->object_id, &property_address, 0, NULL, &size, tmp);
> +        err = AudioObjectGetPropertyData(u->object_id, &property_address, 0, NULL, &size, &tmp_cfstr);
> +        if (err == 0) {
> +            err = !(CFString_to_cstr_n(tmp_cfstr, tmp, sizeof(tmp)));
> +        }
> +
>           if (err || !strlen(tmp))
>               snprintf(tmp, sizeof(tmp), "Channel %d", (int) property_address.mElement);
>
> @@ -505,6 +543,7 @@ static int ca_device_create_source(pa_module *m, AudioBuffer *buf, int channel_i
>       char tmp[255];
>       pa_strbuf *strbuf;
>       AudioObjectPropertyAddress property_address;
> +    CFStringRef tmp_cfstr;
>
>       ca_source = pa_xnew0(coreaudio_source, 1);
>       ca_source->map.channels = buf->mNumberChannels;
> @@ -519,7 +558,11 @@ static int ca_device_create_source(pa_module *m, AudioBuffer *buf, int channel_i
>           property_address.mScope = kAudioDevicePropertyScopeInput;
>           property_address.mElement = channel_idx + i + 1;
>           size = sizeof(tmp);
> -        err = AudioObjectGetPropertyData(u->object_id, &property_address, 0, NULL, &size, tmp);
> +        err = AudioObjectGetPropertyData(u->object_id, &property_address, 0, NULL, &size, &tmp_cfstr);
> +        if (err == 0) {
> +            err = !(CFString_to_cstr_n(tmp_cfstr, tmp, sizeof(tmp)));
> +        }
> +
>           if (err || !strlen(tmp))
>               snprintf(tmp, sizeof(tmp), "Channel %d", (int) property_address.mElement);
>
>

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic