[pulseaudio-discuss] [PATCH 2/2] core: Add debug prints for state changes in sinks and sink-inputs.

[David Henningsson]

On 05/24/2012 12:43 PM, Jarkko Suontausta wrote:
> ---
>   src/pulsecore/sink-input.c |    5 +++++
>   src/pulsecore/sink.c       |    6 ++++++
>   2 files changed, 11 insertions(+), 0 deletions(-)

In general I think this is a good idea, but I'm missing corresponding 
changes to sources and source outputs...?

> diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
> index f6f93b8..a45cc8c 100644
> --- a/src/pulsecore/sink-input.c
> +++ b/src/pulsecore/sink-input.c
> @@ -1758,6 +1758,11 @@ void pa_sink_input_set_state_within_thread(pa_sink_input *i, pa_sink_input_state
>       if (state == i->thread_info.state)
>           return;
>
> +    pa_log_debug("About to switch sink-input #%u's (on %s) thread_info.state from %s to %s",
> +                 i->index, i->sink->name,
> +                 pa_sink_input_state_to_string(i->thread_info.state),
> +                 pa_sink_input_state_to_string(state));
> +
>       if ((state == PA_SINK_INPUT_DRAINED || state == PA_SINK_INPUT_RUNNING)&&
>           !(i->thread_info.state == PA_SINK_INPUT_DRAINED || i->thread_info.state != PA_SINK_INPUT_RUNNING))
>           pa_atomic_store(&i->thread_info.drained, 1);
> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
> index e4c343d..b3a5c4f 100644
> --- a/src/pulsecore/sink.c
> +++ b/src/pulsecore/sink.c
> @@ -396,6 +396,7 @@ static int sink_set_state(pa_sink *s, pa_sink_state_t state) {
>
>       pa_assert(s);
>       pa_assert_ctl_context();
> +    pa_log_debug("%s: Switching state from %s to %s", s->name, pa_sink_state_to_string(s->state), pa_sink_state_to_string(state));
>
>       if (s->state == state)
>           return 0;

You might want it in the other order, like this? Makes it more 
consistent with how you do sink-inputs.

       if (s->state == state)
           return 0;
       pa_log_debug("%s: Switching state from %s to %s", s->name, 
pa_sink_state_to_string(s->state), pa_sink_state_to_string(state));

> @@ -2699,6 +2700,11 @@ int pa_sink_process_msg(pa_msgobject *o, int code, void *userdata, int64_t offse
>                   (s->thread_info.state == PA_SINK_SUSPENDED&&  PA_SINK_IS_OPENED(PA_PTR_TO_UINT(userdata))) ||
>                   (PA_SINK_IS_OPENED(s->thread_info.state)&&  PA_PTR_TO_UINT(userdata) == PA_SINK_SUSPENDED);
>

> +            pa_log_debug("%s: About to change thread_info.state from %s to %s",
> +                          s->name,
> +                          pa_sink_state_to_string(s->thread_info.state),
> +                          pa_sink_state_to_string(PA_PTR_TO_UINT(userdata)));
> +

Having an intermediate variable here would be clearer:
   newstate = PA_PTR_TO_UINT(userdata)
...and then use that variable in all relevant places.

>               s->thread_info.state = PA_PTR_TO_UINT(userdata);
>
>               if (s->thread_info.state == PA_SINK_SUSPENDED) {



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic