[pulseaudio-discuss] [PATCH 22/30] sink-input, source-output: Remove format negotiation using the new data struct
Tanu Kaskinen
tanu.kaskinen at linux.intel.com
Thu Jan 16 07:02:48 PST 2014
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
More information about the pulseaudio-discuss
mailing list