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
