Format negotiation happens also in pa_node_put(), so the negotiation that is done using the new data struct is redundant. --- src/modules/module-device-manager.c | 9 ++-- src/modules/module-intended-roles.c | 11 +++-- src/modules/module-stream-restore.c | 7 ++-- src/pulsecore/sink-input.c | 84 ++++--------------------------------- src/pulsecore/sink-input.h | 6 +-- src/pulsecore/source-output.c | 84 ++++--------------------------------- src/pulsecore/source-output.h | 6 +-- 7 files changed, 35 insertions(+), 172 deletions(-) diff --git a/src/modules/module-device-manager.c b/src/modules/module-device-manager.c index 9a9870e..4740051 100644 --- a/src/modules/module-device-manager.c +++ b/src/modules/module-device-manager.c @@ -965,10 +965,8 @@ static pa_hook_result_t sink_input_new_hook_callback(pa_core *c, pa_sink_input_n if (PA_INVALID_INDEX != device_index) { pa_sink *sink; - if ((sink = pa_idxset_get_by_index(u->core->sinks, device_index))) { - if (pa_sink_input_new_data_set_sink(new_data, sink, false) < 0) - pa_log_debug("Not restoring device for stream because no supported format was found"); - } + if ((sink = pa_idxset_get_by_index(u->core->sinks, device_index))) + pa_sink_input_new_data_set_sink(new_data, sink, false); } } } @@ -1006,8 +1004,7 @@ static pa_hook_result_t source_output_new_hook_callback(pa_core *c, pa_source_ou pa_source *source; if ((source = pa_idxset_get_by_index(u->core->sources, device_index))) - if (pa_source_output_new_data_set_source(new_data, source, false) < 0) - pa_log_debug("Not restoring device for stream because no supported format was found"); + pa_source_output_new_data_set_source(new_data, source, false); } } } diff --git a/src/modules/module-intended-roles.c b/src/modules/module-intended-roles.c index a8cb862..19bee2d 100644 --- a/src/modules/module-intended-roles.c +++ b/src/modules/module-intended-roles.c @@ -94,9 +94,12 @@ static pa_hook_result_t sink_input_new_hook_callback(pa_core *c, pa_sink_input_n } /* Prefer the default sink over any other sink, just in case... */ - if ((def = pa_namereg_get_default_sink(c))) - if (role_match(def->proplist, role) && pa_sink_input_new_data_set_sink(new_data, def, false) >= 0) + if ((def = pa_namereg_get_default_sink(c))) { + if (role_match(def->proplist, role)) { + pa_sink_input_new_data_set_sink(new_data, def, false); return PA_HOOK_OK; + } + } /* @todo: favour the highest priority device, not the first one we find? */ PA_IDXSET_FOREACH(s, c->sinks, idx) { @@ -106,8 +109,10 @@ static pa_hook_result_t sink_input_new_hook_callback(pa_core *c, pa_sink_input_n if (!PA_SINK_IS_LINKED(pa_sink_get_state(s))) continue; - if (role_match(s->proplist, role) && pa_sink_input_new_data_set_sink(new_data, s, false) >= 0) + if (role_match(s->proplist, role)) { + pa_sink_input_new_data_set_sink(new_data, s, false); return PA_HOOK_OK; + } } return PA_HOOK_OK; diff --git a/src/modules/module-stream-restore.c b/src/modules/module-stream-restore.c index 3e50080..33e04f6 100644 --- a/src/modules/module-stream-restore.c +++ b/src/modules/module-stream-restore.c @@ -1441,9 +1441,10 @@ static pa_hook_result_t sink_input_new_hook_callback(pa_core *c, pa_sink_input_n /* It might happen that a stream and a sink are set up at the same time, in which case we want to make sure we don't interfere with that */ - if (s && PA_SINK_IS_LINKED(pa_sink_get_state(s))) - if (pa_sink_input_new_data_set_sink(new_data, s, true) >= 0) - pa_log_info("Restoring device for stream %s.", name); + if (s && PA_SINK_IS_LINKED(pa_sink_get_state(s))) { + pa_log_info("Restoring device for stream %s.", name); + pa_sink_input_new_data_set_sink(new_data, s, true); + } entry_free(e); } diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c index 80c5081..cecdf3e 100644 --- a/src/pulsecore/sink-input.c +++ b/src/pulsecore/sink-input.c @@ -173,40 +173,15 @@ void pa_sink_input_new_data_set_muted(pa_sink_input_new_data *data, bool mute) { data->muted = !!mute; } -int pa_sink_input_new_data_set_sink(pa_sink_input_new_data *data, pa_sink *s, bool save) { - int ret = 0; - pa_idxset *formats = NULL; - +void pa_sink_input_new_data_set_sink(pa_sink_input_new_data *data, pa_sink *s, bool save) { pa_assert(data); pa_assert(s); - if (!data->req_formats) { - /* We're not working with the extended API */ - data->sink = s; - data->save_sink = save; - } else { - /* Extended API: let's see if this sink supports the formats the client can provide */ - formats = pa_sink_check_formats(s, data->req_formats); - - if (formats && !pa_idxset_isempty(formats)) { - /* Sink supports at least one of the requested formats */ - data->sink = s; - data->save_sink = save; - if (data->nego_formats) - pa_idxset_free(data->nego_formats, (pa_free_cb_t) pa_format_info_free); - data->nego_formats = formats; - } else { - /* Sink doesn't support any of the formats requested by the client */ - if (formats) - pa_idxset_free(formats, (pa_free_cb_t) pa_format_info_free); - ret = -PA_ERR_NOTSUPPORTED; - } - } - - return ret; + data->sink = s; + data->save_sink = save; } -int pa_sink_input_new_data_set_formats(pa_sink_input_new_data *data, pa_idxset *formats) { +void pa_sink_input_new_data_set_formats(pa_sink_input_new_data *data, pa_idxset *formats) { pa_assert(data); pa_assert(formats); @@ -214,13 +189,6 @@ int pa_sink_input_new_data_set_formats(pa_sink_input_new_data *data, pa_idxset * pa_idxset_free(formats, (pa_free_cb_t) pa_format_info_free); data->req_formats = formats; - - if (data->sink) { - /* Trigger format negotiation */ - return pa_sink_input_new_data_set_sink(data, data->sink, data->save_sink); - } - - return 0; } void pa_sink_input_new_data_done(pa_sink_input_new_data *data) { @@ -231,12 +199,6 @@ void pa_sink_input_new_data_done(pa_sink_input_new_data *data) { if (data->req_formats) pa_idxset_free(data->req_formats, (pa_free_cb_t) pa_format_info_free); - if (data->nego_formats) - pa_idxset_free(data->nego_formats, (pa_free_cb_t) pa_format_info_free); - - if (data->format) - pa_format_info_free(data->format); - if (data->volume_factor_items) pa_hashmap_free(data->volume_factor_items); @@ -338,28 +300,8 @@ int pa_sink_input_new( if (data->sink) sink_node = pa_sink_get_node(data->sink); - /* If something didn't pick a format for us, pick the top-most format since - * we assume this is sorted in priority order */ - if (!data->format && data->nego_formats && !pa_idxset_isempty(data->nego_formats)) - data->format = pa_format_info_copy(pa_idxset_first(data->nego_formats, NULL)); - - /* If the sink has been set, then also the format should have been - * negotiated. If the sink hasn't been set, no format negotiation should - * have happened either. */ - pa_assert((data->sink && data->format) || (!data->sink && !data->format)); - - /* From now on, routing will be done with the sink input object instead of - * the new data, because the routing code that is in pa_node_put() doesn't - * have access to the new data. Therefore, let's copy data->req_formats, - * data->format and data->sink to i, because the routing code needs access - * to those. */ i->req_formats = pa_idxset_copy(data->req_formats, (pa_copy_func_t) pa_format_info_copy); - if (data->format) - i->format = pa_format_info_copy(data->format); - - i->sink = data->sink; - if (data->driver && !pa_utf8_valid(data->driver)) { ret = -PA_ERR_INVALID; goto fail; @@ -378,7 +320,7 @@ int pa_sink_input_new( i->node->owner = i; - /* This may update i->sink and i->format. */ + /* This will set i->sink and i->format. */ r = pa_node_put(i->node, &sink_node, sink_node ? 1 : 0); if (r < 0) { @@ -389,21 +331,11 @@ int pa_sink_input_new( pa_assert(i->sink); pa_assert(i->format); - /* Modules may want to look at the sink and format in the FIXATE hook, so - * let's make sure that the new data is in sync with the sink input. Also, - * pa_sink_input_new_data_is_passthrough() uses data->format too, and we - * call that function a few times. (XXX: It would probably be better to - * pass the sink input object to the FIXATE hook instead of the new data, - * and replace the pa_sink_input_new_data_is_passthrough() usage with - * something that uses the sink input object instead of the new data. Then - * this extra syncing could be removed.) */ + /* Modules may want to access the sink in the FIXATE hook, so let's make + * the sink available in the new data. FIXME: It would be cleaner to just + * pass the real sink input object to the FIXATE hook. */ data->sink = i->sink; - if (data->format) - pa_format_info_free(data->format); - - data->format = pa_format_info_copy(i->format); - /* Routing's done, we have a sink and a format. Now populate the sample * spec and channel map according to the final format that we've * negotiated. */ diff --git a/src/pulsecore/sink-input.h b/src/pulsecore/sink-input.h index 97663a9..9c29408 100644 --- a/src/pulsecore/sink-input.h +++ b/src/pulsecore/sink-input.h @@ -298,9 +298,7 @@ typedef struct pa_sink_input_new_data { pa_sample_spec sample_spec; pa_channel_map channel_map; - pa_format_info *format; pa_idxset *req_formats; - pa_idxset *nego_formats; pa_cvolume volume; bool muted:1; @@ -328,8 +326,8 @@ void pa_sink_input_new_data_set_volume(pa_sink_input_new_data *data, const pa_cv void pa_sink_input_new_data_add_volume_factor(pa_sink_input_new_data *data, const char *key, const pa_cvolume *volume_factor); void pa_sink_input_new_data_add_volume_factor_sink(pa_sink_input_new_data *data, const char *key, const pa_cvolume *volume_factor); void pa_sink_input_new_data_set_muted(pa_sink_input_new_data *data, bool mute); -int pa_sink_input_new_data_set_sink(pa_sink_input_new_data *data, pa_sink *s, bool save); -int pa_sink_input_new_data_set_formats(pa_sink_input_new_data *data, pa_idxset *formats); +void pa_sink_input_new_data_set_sink(pa_sink_input_new_data *data, pa_sink *s, bool save); +void pa_sink_input_new_data_set_formats(pa_sink_input_new_data *data, pa_idxset *formats); void pa_sink_input_new_data_done(pa_sink_input_new_data *data); /* To be called by the implementing module only */ diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c index 6a792ec..83e3715 100644 --- a/src/pulsecore/source-output.c +++ b/src/pulsecore/source-output.c @@ -117,40 +117,15 @@ void pa_source_output_new_data_set_muted(pa_source_output_new_data *data, bool m data->muted = !!mute; } -int pa_source_output_new_data_set_source(pa_source_output_new_data *data, pa_source *s, bool save) { - int ret = 0; - pa_idxset *formats = NULL; - +void pa_source_output_new_data_set_source(pa_source_output_new_data *data, pa_source *s, bool save) { pa_assert(data); pa_assert(s); - if (!data->req_formats) { - /* We're not working with the extended API */ - data->source = s; - data->save_source = save; - } else { - /* Extended API: let's see if this source supports the formats the client would like */ - formats = pa_source_check_formats(s, data->req_formats); - - if (formats && !pa_idxset_isempty(formats)) { - /* Source supports at least one of the requested formats */ - data->source = s; - data->save_source = save; - if (data->nego_formats) - pa_idxset_free(data->nego_formats, (pa_free_cb_t) pa_format_info_free); - data->nego_formats = formats; - } else { - /* Source doesn't support any of the formats requested by the client */ - if (formats) - pa_idxset_free(formats, (pa_free_cb_t) pa_format_info_free); - ret = -PA_ERR_NOTSUPPORTED; - } - } - - return ret; + data->source = s; + data->save_source = save; } -int pa_source_output_new_data_set_formats(pa_source_output_new_data *data, pa_idxset *formats) { +void pa_source_output_new_data_set_formats(pa_source_output_new_data *data, pa_idxset *formats) { pa_assert(data); pa_assert(formats); @@ -158,13 +133,6 @@ int pa_source_output_new_data_set_formats(pa_source_output_new_data *data, pa_id pa_idxset_free(formats, (pa_free_cb_t) pa_format_info_free); data->req_formats = formats; - - if (data->source) { - /* Trigger format negotiation */ - return pa_source_output_new_data_set_source(data, data->source, data->save_source); - } - - return 0; } void pa_source_output_new_data_done(pa_source_output_new_data *data) { @@ -175,12 +143,6 @@ void pa_source_output_new_data_done(pa_source_output_new_data *data) { if (data->req_formats) pa_idxset_free(data->req_formats, (pa_free_cb_t) pa_format_info_free); - if (data->nego_formats) - pa_idxset_free(data->nego_formats, (pa_free_cb_t) pa_format_info_free); - - if (data->format) - pa_format_info_free(data->format); - pa_proplist_free(data->proplist); } @@ -271,28 +233,8 @@ int pa_source_output_new( if (data->source) source_node = pa_source_get_node(data->source); - /* If something didn't pick a format for us, pick the top-most format since - * we assume this is sorted in priority order */ - if (!data->format && data->nego_formats && !pa_idxset_isempty(data->nego_formats)) - data->format = pa_format_info_copy(pa_idxset_first(data->nego_formats, NULL)); - - /* If the source has been set, then also the format should have been - * negotiated. If the source hasn't been set, no format negotiation should - * have happened either. */ - pa_assert((data->source && data->format) || (!data->source && !data->format)); - - /* From now on, routing will be done with the source output object instead - * of the new data, because the routing code that is in pa_node_put() - * doesn't have access to the new data. Therefore, let's copy - * data->req_formats, data->format and data->source to o, because the - * routing code needs access to those. */ o->req_formats = pa_idxset_copy(data->req_formats, (pa_copy_func_t) pa_format_info_copy); - if (data->format) - o->format = pa_format_info_copy(data->format); - - o->source = data->source; - if (data->driver && !pa_utf8_valid(data->driver)) { ret = -PA_ERR_INVALID; goto fail; @@ -311,7 +253,7 @@ int pa_source_output_new( o->node->owner = o; - /* This may update o->source and o->format. */ + /* This will set o->source and o->format. */ r = pa_node_put(o->node, &source_node, source_node ? 1 : 0); if (r < 0) { @@ -322,21 +264,11 @@ int pa_source_output_new( pa_assert(o->source); pa_assert(o->format); - /* Modules may want to look at the source and format in the FIXATE hook, so - * let's make sure that the new data is in sync with the source output. - * Also, pa_source_output_new_data_is_passthrough() uses data->format too, - * and we call that function a few times. (XXX: It would probably be better - * to pass the source output object to the FIXATE hook instead of the new - * data, and replace the pa_source_output_new_data_is_passthrough() usage - * with something that uses the source output object instead of the new - * data. Then this extra syncing could be removed.) */ + /* Modules may want to access the source in the FIXATE hook, so let's make + * the source available in the new data. FIXME: It would be cleaner to just + * pass the real source output object to the FIXATE hook. */ data->source = o->source; - if (data->format) - pa_format_info_free(data->format); - - data->format = pa_format_info_copy(o->format); - /* Routing's done, we have a source and a format. Now populate the sample * spec and channel map according to the final format that we've * negotiated. */ diff --git a/src/pulsecore/source-output.h b/src/pulsecore/source-output.h index a2d4474..8ce8b6d 100644 --- a/src/pulsecore/source-output.h +++ b/src/pulsecore/source-output.h @@ -255,9 +255,7 @@ typedef struct pa_source_output_new_data { pa_sample_spec sample_spec; pa_channel_map channel_map; - pa_format_info *format; pa_idxset *req_formats; - pa_idxset *nego_formats; pa_cvolume volume, volume_factor, volume_factor_source; bool muted:1; @@ -284,8 +282,8 @@ void pa_source_output_new_data_set_volume(pa_source_output_new_data *data, const void pa_source_output_new_data_apply_volume_factor(pa_source_output_new_data *data, const pa_cvolume *volume_factor); void pa_source_output_new_data_apply_volume_factor_source(pa_source_output_new_data *data, const pa_cvolume *volume_factor); void pa_source_output_new_data_set_muted(pa_source_output_new_data *data, bool mute); -int pa_source_output_new_data_set_source(pa_source_output_new_data *data, pa_source *s, bool save); -int pa_source_output_new_data_set_formats(pa_source_output_new_data *data, pa_idxset *formats); +void pa_source_output_new_data_set_source(pa_source_output_new_data *data, pa_source *s, bool save); +void pa_source_output_new_data_set_formats(pa_source_output_new_data *data, pa_idxset *formats); void pa_source_output_new_data_done(pa_source_output_new_data *data); /* To be called by the implementing module only */ -- 1.8.3.1