Loopback mode implementation #84
Conversation
|
Tagging @ChunMinChang for initial review if he has some cycles. Thanks for the contribution, I'll run CI now. |
|
I am on PTO right now. I can take a look next Monday. |
While not an officially advertized feature in the PulseAudio API, all PulseAudio "sink"-s have a "monitor mode" where in any of the audio output devices there's a "loopback" input device which replicates all audio for said output. This allows us to effectively achieve recording any audio output as input within cubeb. This commit is a continuation of said Draft shown in mozilla/cubeb#691 but this time written in Rust. (yes it took me half a year but we're here) As I'm quite inexperienced when it comes to Rust (*especially so when it's demanding code like FFI interfaces*) comments on the code or any improvements that may be done are greatly appreciated. Thanks.
|
Forced-pushed the "cargo fmt"-ed version as I was not aware of the tool. I would like a CI re-run. |
| ) { | ||
| let r = unsafe { ptr::NonNull::new(u as *mut CallbackData).unwrap().as_mut() }; | ||
|
|
||
| let name = unsafe { CStr::from_ptr(info.unwrap().default_sink_name) }; |
There was a problem hiding this comment.
Does info always Some(_)? What happens if info is None?
There was a problem hiding this comment.
Should be ALWAYS valid, because we control when the callback is called, not the Pulse API. By that I mean me make sure that the PulseAudio API calls our callback ONLY WHEN we want it, otherwise it'd be a massive UB hole to patch.
There was a problem hiding this comment.
Not that familiar with Rust as you can notice, but I assume that the callback function would be invalid if we leave the scope (by that I mean the get_default_sink_name function)?
src/backend/stream.rs
Outdated
| default_sink_name: String::new(), | ||
| }; | ||
|
|
||
| _ = context.get_server_info(server_info_cb, ptr::addr_of!(data) as *mut c_void); |
There was a problem hiding this comment.
This silence the error. I prefer returning an error.
If the loopback functionality still works even if get_server_info fails, assigning a default name to default_sink_name and adding a log here is acceptable for me.
There was a problem hiding this comment.
Yep, will be handled the same as the sink string in a Result<> type.
| actual_stream_name = Some(sink_name.as_c_str()); | ||
| } | ||
|
|
||
| match PulseStream::stream_init(context, stream_params, actual_stream_name) { |
There was a problem hiding this comment.
Does this work for the input-only stream?
This seems to work only for the duplex stream and the input device isn't set?
I am not quite sure what the expected behavior for the following cases:
- the stream is input-only stream
- the stream is duplex stream but the input device is set by user
- the stream is duplex stream but the input device is not set
(IIUC, loopback doesn't work for output-only stream)
@padenot : What are the expected behaviors for the above cases?
There was a problem hiding this comment.
For now, it only fetched whatever default sink device it finds. IN THEORY we could implement it finding any sink device, but I don't think there are many use-cases for such behavior and therefore have avoided it.
Case 1:
For input-only streams, (using the error checking code that you suggested that I make) it will return a cubeb_backend::Error::device_unavailable() because there is no sink device you can grab the source from.
Therefore if the library user tried to request a loopback stream and found none, they should be informed about the fact that there are indeed NO loopback streams available.
Case 2:
That should be invalid (for now, given my first and lazy implementation) but in theory we could make it so it can fetch the .monitor stream of any sink device. Reason being that the code would have to properly parse the input device and activate a sink just so it could grab it's name and use it as a sink. This would require a TON of changes.
Case 3:
It should just grab the default sink device as use it as a source as I've alluded to before.
But yeah, I you want any of these cases to change in behavior, I'd be happy to change. But for now I just made them do whatever my time/intuition could allow for.
src/backend/stream.rs
Outdated
| mainloop.unlock(); | ||
|
|
||
| _ = data.default_sink_name.pop(); | ||
| data.default_sink_name.push_str(".monitor"); |
There was a problem hiding this comment.
I guess the .monitor is the key to this magical setting. Can I have the document for this to check?
There was a problem hiding this comment.
There isn't any official page describing what .monitor does however it's use IS mentioned here and there within PulseAudio's official docs. Example being: https://www.freedesktop.org/wiki/Software/PulseAudio/Documentation/Developer/AccessControl/
|
I've done some changed to add error checking, however handling of selecting what sinks you want to use is still not implemented (because of the complexities involved). |
|
Apologies if any of the mistakes in advance seem trivial. |
While not an officially advertised feature in the PulseAudio API, all PulseAudio "sink"-s have a "monitor mode" where in any of the audio output devices there's a "loopback" input device which replicates all audio for said output. This allows us to effectively achieve recording any audio output as input within cubeb.
This commit is a continuation of said Draft shown in mozilla/cubeb#691 but this time written in Rust. (yes it took me half a year but we're here)
ps: rust is hardAs I'm quite inexperienced when it comes to Rust (especially so when it's demanding code like FFI) comments on the code or any improvements that may be done are greatly appreciated.
Thanks.