Conversation
Introduce the audio emulation pipeline for the Cuttlefish Virtual Tuner: - AudioGenerator: Generates 48 kHz, 16-bit stereo PCM audio frames (white noise while tuned/playing, silence while stopped/untuned). - PcmStreamServer: Multi-client PCM stream server delivering 48 kHz 16-bit stereo audio frames over a UNIX domain stream socket to Cuttlefish's virtio-snd virtual sound card backend (CrosVM). - Unit and integration tests covering audio frame synthesis and socket streaming. Test: bazel test //cuttlefish/host/commands/virtual_tuner_daemon:... Bug: 521329213 TAG=agy CONV=809829e8-ac68-4c32-b9b7-632f61743e47
jemoreira
left a comment
jemoreira
left a comment
There was a problem hiding this comment.
Please rebase your branch on top of main instead of creating merge commits.
| void StreamClient(ClientSession* session); | ||
| void ReapFinishedClients(); | ||
|
|
||
| TunerState* const tuner_state_; |
| if (written > 0) { | ||
| cursor += written; | ||
| remaining -= static_cast<size_t>(written); | ||
| } else if (written < 0 && fd->GetErrno() == EINTR) { |
There was a problem hiding this comment.
Fd::Send handles EINTR for you, this check is unnecessary.
| constexpr auto kMaxSchedulingLag = std::chrono::milliseconds(500); | ||
| constexpr int kPrebufferChunks = 2; | ||
|
|
||
| bool SendAll(const SharedFD& fd, const void* data, size_t size) { |
There was a problem hiding this comment.
There is already a SendAll function in common/libs/fs/shared_buf.*, please add an overload that takes void*, size_t instead of std::string_view and have the existing one call the new one to avoid repetition.
While you're at it, please add the flags parameter as well with a default value of MSG_NOSIGNAL to make it more flexible without changing the current behavior.
| } | ||
|
|
||
| ::unlink(socket_path_.c_str()); | ||
| server_fd_ = SharedFD::SocketLocalServer( |
There was a problem hiding this comment.
Because of the way cuttlefish spawns processes there is no guarantee that this socket will be created before the client attempts to connect to it. Our way of dealing with this race is to have the parent process run_cvd create the socket and let the child process (this one) inherit it. The actual fd is passed by command line parameter.
There are several examples under host/commands/run_cvd/launch that you can use for guidance.
| } | ||
|
|
||
| if (server_fd_->IsOpen()) { | ||
| server_fd_->Shutdown(SHUT_RDWR); |
There was a problem hiding this comment.
why Shutdown? Is Close not enough?
|
|
||
| LOG(INFO) << "PCM streaming server accepted a client."; | ||
| auto session = std::make_unique<ClientSession>(); | ||
| session->fd = std::move(client_fd); |
There was a problem hiding this comment.
nit: client_fd is a SharedFD, it's typically copied instead of moved, just like a shared_ptr.
| }); | ||
| } | ||
|
|
||
| void PcmStreamServer::StreamClient(ClientSession* session) { |
There was a problem hiding this comment.
nit: use ClientSession& for the parameter
| std::this_thread::sleep_until(deadline); | ||
| if (!is_running_) { |
There was a problem hiding this comment.
nit: If you use std::condition_variable::wait_until you can notify the variable from the stop routine and not have to wait until this sleep ends.
| AudioGenerator generator; | ||
| std::vector<int16_t> buffer(kChunkFrames * kChannels); | ||
|
|
||
| const auto write_next_chunk = [&] { |
There was a problem hiding this comment.
please write the type for this lambda since it's not obvious that it returns bool without looking at other source locations.
| namespace virtualtuner { | ||
| namespace { | ||
|
|
||
| bool ReadExactly(const SharedFD& fd, std::span<uint8_t> buffer) { |
There was a problem hiding this comment.
We already have ReadExact in shared_buf.h
2 participants
Introduce the audio emulation pipeline for the Cuttlefish Virtual Tuner:
Test: bazel test //cuttlefish/host/commands/virtual_tuner_daemon:...
Bug: 521329213
TAG=agy
CONV=809829e8-ac68-4c32-b9b7-632f61743e47