On 13.03.2018 18:40, Tanu Kaskinen wrote:
The previous code made the SET_STATE message fail if trigger() failed.
However, trigger() was called after pa_sink/source_process_msg(), which
meant that the main thread that sent the SET_STATE thought that resuming
failed, but nothing was undone in the IO thread, so in the IO thread
things seemed as if the sink/source was successfully resumed. (I don't
use OSS myself, so I don't know what kind of practical problems this
could cause).

Unless some complex undo logic is implemented, I believe it's best to
ignore all failures in trigger(). Most error cases were already ignored,
and the only one that wasn't ignored doesn't seem too serious.

I also moved trigger() to happen before pa_sink/source_process_msg(),
which made it necessary to add new state parameters to trigger(). The
reason for this move is that I want to move the SET_STATE handler code
into a separate callback, and if things are done both before and after
pa_sink/source_process_msg(), that makes things more complicated.

The previous code checked the return value of
pa_sink/source_process_msg() before calling trigger(), but that was
unnecessary, since pa_sink/source_process_msg() never fails when
processing the SET_STATE messages.
---
  src/modules/oss/module-oss.c | 59 ++++++++++++++++++++------------------------
  1 file changed, 27 insertions(+), 32 deletions(-)

LGTM
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to