On Sun, 2012-06-17 at 14:47 +0200, poljar (Damir Jelic) wrote:
> From: poljar <poljarinho at gmail.com>
>
> A latency offset variable was added to the port struct and functions to
> for handling a custom latency.
>
> We can now attach a latency to the port which will be added to the
> sink/source latency if the port is active.
> ---
> src/pulsecore/device-port.c | 34 ++++++++++++++++++++++++++++++++++
> src/pulsecore/device-port.h | 6 ++++++
> 2 files changed, 40 insertions(+)
>
> diff --git a/src/pulsecore/device-port.c b/src/pulsecore/device-port.c
> index 5870913..c5639b8 100644
> --- a/src/pulsecore/device-port.c
> +++ b/src/pulsecore/device-port.c
> @@ -97,6 +97,7 @@ pa_device_port *pa_device_port_new(pa_core *c, const char *name, const char *des
> p->profiles = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
> p->is_input = FALSE;
> p->is_output = FALSE;
> + p->latency_offset = 0;
> p->proplist = pa_proplist_new();
>
> return p;
> @@ -112,3 +113,36 @@ void pa_device_port_hashmap_free(pa_hashmap *h) {
>
> pa_hashmap_free(h, NULL, NULL);
> }
> +
> +void pa_device_port_set_latency_offset(pa_device_port *p, pa_usec_t latency) {
I think "offset" would be a more descriptive parameter name than
"latency", because the offset is not really a stand-alone latency value.
> + uint32_t state;
> + union {
> + pa_sink *sink;
> + pa_source *source;
> + } data;
> +
> + pa_assert(p);
> +
> + p->latency_offset = latency;
> +
> + if (p->is_output) {
The sink pointer could be declared here, because it's not used outside
this if block. That would avoid the need for the union.
> + PA_IDXSET_FOREACH(data.sink, p->core->sinks, state)
> + if (data.sink->active_port == p) {
> + pa_sink_set_latency_offset(data.sink, p->latency_offset);
It would be good to keep the code compilable in each patch. Here
pa_sink_set_latency_offset() is used before it exists, so the
compilation breaks. This matters at least when doing bisecting.
Maybe the most straight-forward way of fixing this would be to squash
the three first patches together.
> + break;
> + }
> +
> + } else {
> + PA_IDXSET_FOREACH(data.source, p->core->sources, state)
> + if (data.source->active_port == p) {
> + pa_source_set_latency_offset(data.source, p->latency_offset);
> + break;
> + }
Misaligned closing bracket.
> + }
> +}
> +
> +pa_usec_t pa_device_port_get_latency_offset(pa_device_port *p) {
> + pa_assert(p);
> +
> + return p->latency_offset;
> +}
We tend not to have trivial getter functions in the server-side APIs,
unless the struct definition is hidden (pa_device_port definition is not
hidden). Trivial setter functions are fine, at least in my opinion, and
I even recommend using them (I don't like when component state is
directly changed from code outside that component).
--
Tanu
