genodelabs / genode

Genode OS Framework
https://genode.org/
Other
1.03k stars 249 forks source link

record_play_mixer: periodic warning after stop() called #5175

Closed alex-ab closed 3 weeks ago

alex-ab commented 1 month ago

After temporarily calling stop(), the mixer seems still to operate the sessions and start complaining in the Sculpt log continuously:

[init -> mixer] Warning: Play (seoul -> left) required sample value is no longer available
[init -> mixer] Warning: Play (seoul -> left) (jitter config or period too high?)

even so the client announced, that no new data will be enqueued.

Can/should the _stopped() method check also be applied to the warning case, as done for the later_than_avail_samples case, or should the processing be stopped already earlier (saving CPU time) ?

https://github.com/genodelabs/genode/blob/master/repos/os/src/server/record_play_mixer/play_session.h#L327

if (probe_result == Probe_result::MISSING) {
    if (earlier_than_avail_samples) {
        warning("required sample value is no longer available");
        warning("(jitter config or period too high?)");
    }
    else if (later_than_avail_samples && !_stopped()) {
        warning("required sample is not yet available");
        warning("(increase 'jitter_ms' config attribute?)");
    }
}
nfeske commented 1 month ago

Thanks for bringing this behavior to my attention. Your suggestion to apply the _stopped to both branches (moving it to the compounding if condition) is sensible. Even though the message is prefixes with the play session, it is triggered in the context of the record client's interaction with the mixer (when trying to obtain the sample data for mixing). When the condition of no more play data occurs, the depletion to reflected to the record client, which can use it to stop recording.

chelmuth commented 1 month ago

Does 3af3773 fix this issue?

alex-ab commented 1 month ago

@chelmuth: Yes.

chelmuth commented 3 weeks ago

Fix merged to master.