On Tue, 2015-12-29 at 09:03 +0530, arun at accosted.net wrote:
> +void pa_command_stream_volume_changed(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_tagstruct *t, void *userdata) {
> +Â Â Â Â pa_context *c = userdata;
> +Â Â Â Â pa_stream *s;
> +Â Â Â Â uint32_t index;
> +Â Â Â Â pa_cvolume volume;
> +
> +Â Â Â Â pa_assert(pd);
> +Â Â Â Â pa_assert(command == PA_COMMAND_PLAYBACK_STREAM_VOLUME_CHANGED || command == PA_COMMAND_RECORD_STREAM_VOLUME_CHANGED);
> +Â Â Â Â pa_assert(t);
> +Â Â Â Â pa_assert(c);
> +Â Â Â Â pa_assert(PA_REFCNT_VALUE(c) >= 1);
> +
> +Â Â Â Â pa_context_ref(c);
> +
> +Â Â Â Â if (c->version < 31) {
> +Â Â Â Â Â Â Â Â pa_context_fail(c, PA_ERR_PROTOCOL);
> +Â Â Â Â Â Â Â Â goto finish;
> +Â Â Â Â }
> +
> +Â Â Â Â if (pa_tagstruct_getu32(t, &index) < 0 ||
> +Â Â Â Â Â Â Â Â pa_tagstruct_get_cvolume(t, &volume) < 0 ||
> +Â Â Â Â Â Â Â Â !pa_tagstruct_eof(t)) {
> +Â Â Â Â Â Â Â Â pa_context_fail(c, PA_ERR_PROTOCOL);
> +Â Â Â Â Â Â Â Â goto finish;
> +Â Â Â Â }
> +
> +Â Â Â Â if (!(s = pa_hashmap_get(command == PA_COMMAND_PLAYBACK_STREAM_VOLUME_CHANGED ? c->playback_streams : c->record_streams,
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â PA_UINT32_TO_PTR(index))))
> +Â Â Â Â Â Â Â Â goto finish;
> +
> +Â Â Â Â s->volume = volume;
We should invoke protocol error if s is an upload stream (upload
streams are stored in c->playback_streams).
> +
> +finish:
> +Â Â Â Â pa_context_unref(c);
> +}
> +
> Â static void invalidate_indexes(pa_stream *s, bool r, bool w) {
> Â Â Â Â Â pa_assert(s);
> Â Â Â Â Â pa_assert(PA_REFCNT_VALUE(s) >= 1);
> @@ -1154,6 +1190,9 @@ void pa_create_stream_callback(pa_pdispatch *pd, uint32_t command, uint32_t tag,
> Â Â Â Â Â Â Â Â Â }
> Â Â Â Â Â }
> Â
> +Â Â Â Â if (s->context->version >= 31)
> +Â Â Â Â Â Â Â Â pa_tagstruct_get_cvolume(t, &s->volume);
pa_tagstruct_get_cvolume() return value should be checked.
Also, I think upload streams shouldn't have volume associated with
them.
Also, s->volume_set needs to be set.
> +
> Â Â Â Â Â if (!pa_tagstruct_eof(t)) {
> Â Â Â Â Â Â Â Â Â pa_context_fail(s->context, PA_ERR_PROTOCOL);
> Â Â Â Â Â Â Â Â Â goto finish;
> @@ -2968,3 +3007,19 @@ int pa_stream_set_volume(pa_stream *s, pa_cvolume *v, pa_stream_success_cb_t cb,
> Â
> Â Â Â Â Â return 0;
> Â }
> +
> +int pa_stream_get_volume(pa_stream *s, pa_cvolume *v) {
> +Â Â Â Â pa_assert(s);
> +Â Â Â Â pa_assert(v);
> +Â Â Â Â pa_assert(PA_REFCNT_VALUE(s) >= 1);
> +
> +Â Â Â Â PA_CHECK_VALIDITY(s->context, !pa_detect_fork(), PA_ERR_FORKED);
> +Â Â Â Â PA_CHECK_VALIDITY(s->context, s->state != PA_STREAM_FAILED && s->state != PA_STREAM_TERMINATED, PA_ERR_BADSTATE);
> +Â Â Â Â PA_CHECK_VALIDITY(s->context, s->state == PA_STREAM_READY || s->volume_set, PA_ERR_NODATA);
Shouldn't it be enough to check only s->volume_set? If the stream is
ready, then the volume should be always set.
Also, I think this should be the last check. If multiple error
conditions exist, PA_ERR_NODATA should have the lowest priority.
> +Â Â Â Â PA_CHECK_VALIDITY(s->context, s->direction != PA_STREAM_UPLOAD, PA_ERR_INVALID); /* TODO: do we want to support this? */
No, we don't.
> +Â Â Â Â PA_CHECK_VALIDITY(s->context, s->context->version >= 31, PA_ERR_NOTSUPPORTED);
> +
> +Â Â Â Â *v = s->volume;
> +
> +Â Â Â Â return 0;
> +}
> diff --git a/src/pulse/stream.h b/src/pulse/stream.h
> index cc74629..b4e6ff9 100644
> --- a/src/pulse/stream.h
> +++ b/src/pulse/stream.h
> @@ -828,6 +828,13 @@ uint32_t pa_stream_get_monitor_stream(pa_stream *s);
> Â * \since 9.0 */
> Â int pa_stream_set_volume(pa_stream *s, pa_cvolume *v, pa_stream_success_cb_t cb, void *userdata);
> Â
> +/** Get the volume on the given stream.
> + *
> + * Returns 0 on success, negative error value on failure.
> + *
> + * \since 9.0 */
> +int pa_stream_get_volume(pa_stream *s, pa_cvolume *v);
It should be documented that this is only supported starting from
protocol version 31.
> +
> Â PA_C_DECL_END
> Â
> Â #endif
> diff --git a/src/pulsecore/native-common.h b/src/pulsecore/native-common.h
> index dc62895..73aded1 100644
> --- a/src/pulsecore/native-common.h
> +++ b/src/pulsecore/native-common.h
> @@ -179,6 +179,11 @@ enum {
> Â Â Â Â Â PA_COMMAND_ENABLE_SRBCHANNEL,
> Â Â Â Â Â PA_COMMAND_DISABLE_SRBCHANNEL,
> Â
> +Â Â Â Â /* Supported since protocol v31 (8.0) */
Should be 9.0.
> +Â Â Â Â /* SERVER->CLIENT */
> +Â Â Â Â PA_COMMAND_PLAYBACK_STREAM_VOLUME_CHANGED,
> +Â Â Â Â PA_COMMAND_RECORD_STREAM_VOLUME_CHANGED,
--
Tanu
