[pulseaudio-discuss] [RFC] disabling monitor source patch

[Tanu Kaskinen]

On Wed, 2010-11-03 at 09:35 -0500, pl bossart wrote:
> I started disabling the monitor sources, but I found out some isses
> with the specific modules (dbus interface, http protocol, rygel
> server), where I could use some help. I have no idea what these pieces
> of code do and what the behavior should be if there is no monitor
> source for a specific sink. These isssues are highlighted with FIXME
> in the code.

I'll comment for the dbus interface parts.

> @@ -979,9 +979,12 @@ static void handle_sink_get_monitor_source(DBusConnection *conn, DBusMessage *ms
>      pa_assert(d);
>      pa_assert(d->type == DEVICE_TYPE_SINK);
>  
> -    monitor_source = pa_dbusiface_core_get_source_path(d->core, d->sink->monitor_source);
> +    /* FIXME: what is the expected behavior when there's no monitor ? */
> +    if (d->sink->monitor_source) {
> +        monitor_source = pa_dbusiface_core_get_source_path(d->core, d->sink->monitor_source);
>  
> -    pa_dbus_send_basic_variant_reply(conn, msg, DBUS_TYPE_OBJECT_PATH, &monitor_source);
> +        pa_dbus_send_basic_variant_reply(conn, msg, DBUS_TYPE_OBJECT_PATH, &monitor_source);
> +    }
>  }

If there's no monitor, then throw
org.PulseAudio.Core1.NoSuchPropertyError. There are many other instances
where some property may not always exist, see
handle_get_active_profile() in iface-card.c, for example:
http://git.0pointer.de/?p=pulseaudio.git;a=blob;f=src/modules/dbus/iface-card.c;h=d99c8b95b286c6a6c100d15c615858c8931420c7;hb=HEAD#l304

> @@ -996,20 +999,23 @@ static void handle_sink_get_all(DBusConnection *conn, DBusMessage *msg, void *us
>      pa_assert(d);
>      pa_assert(d->type == DEVICE_TYPE_SINK);
>  
> -    monitor_source = pa_dbusiface_core_get_source_path(d->core, d->sink->monitor_source);
> +    /* FIXME: what is the expected behavior when there's no monitor ? */
> +    if (d->sink->monitor_source) {
> +        monitor_source = pa_dbusiface_core_get_source_path(d->core, d->sink->monitor_source);
>  
> -    pa_assert_se((reply = dbus_message_new_method_return(msg)));
> +        pa_assert_se((reply = dbus_message_new_method_return(msg)));
>  
> -    dbus_message_iter_init_append(reply, &msg_iter);
> -    pa_assert_se(dbus_message_iter_open_container(&msg_iter, DBUS_TYPE_ARRAY, "{sv}", &dict_iter));
> +        dbus_message_iter_init_append(reply, &msg_iter);
> +        pa_assert_se(dbus_message_iter_open_container(&msg_iter, DBUS_TYPE_ARRAY, "{sv}", &dict_iter));
>  
> -    pa_dbus_append_basic_variant_dict_entry(&dict_iter, property_handlers[SINK_PROPERTY_HANDLER_MONITOR_SOURCE].property_name, DBUS_TYPE_OBJECT_PATH, &monitor_source);
> +        pa_dbus_append_basic_variant_dict_entry(&dict_iter, property_handlers[SINK_PROPERTY_HANDLER_MONITOR_SOURCE].property_name, DBUS_TYPE_OBJECT_PATH, &monitor_source);
>  
> -    pa_assert_se(dbus_message_iter_close_container(&msg_iter, &dict_iter));
> +        pa_assert_se(dbus_message_iter_close_container(&msg_iter, &dict_iter));
>  
> -    pa_assert_se(dbus_connection_send(conn, reply, NULL));
> +        pa_assert_se(dbus_connection_send(conn, reply, NULL));
>  
> -    dbus_message_unref(reply);
> +        dbus_message_unref(reply);
> +    }
>  }

See handle_source_get_all(), it has the exact same situation with the
MonitorOfSink property. I guess handle_source_get_monitor_of_sink()
would have been better example for the previous item, or paragraph, or
whatever, you know, the place where I was commenting on
handle_sink_get_monitor_source() implementation... But hey, two examples
are better than one anyway!

-- 
Tanu Kaskinen