search

rust-vmm / vhost-device

'vhost-user' device backends workspace
Apache License 2.0
67 stars 46 forks source link

[sound] add a comment to explain latency value #501

Open MatiasVara opened 10 months ago

MatiasVara commented 10 months ago

Based on crosvm comment, the latency of an used buffer is the number of bytes before the current buffer is played:

/// It returns how many bytes need to be consumed
/// before the current playback buffer will be played.

My guess is that Crosvm is using "0" because it notifies the guest after the current period has been consumed. We could add a comment to explain that too at
https://github.com/rust-vmm/vhost-device/blob/573e592037d971058eda39d6f04ae2322d8f0701/staging/vhost-device-sound/src/lib.rs#L261

epilys commented 10 months ago

https://github.com/rust-vmm/vhost-device/blob/573e592037d971058eda39d6f04ae2322d8f0701/staging/vhost-device-sound/src/lib.rs#L261

The doc comment can be similar to the commit that introduced the field:

sound: add latency_bytes field to IOMessage

The latency field specifies how long it will take until the device finishes playing the I/O message's buffers.

Source:

https://lists.oasis-open.org/archives/virtio-dev/201912/msg00123.html

MatiasVara commented 10 months ago

Right, it is not clear to me yet how the virtio-sound driver uses latency_bytes value. AFAIU, it seems it is stored to runtime->delay during virtsnd_pcm_msg_complete() but I do not understand what the kernel does with that.

epilys commented 10 months ago

@MatiasVara I had the same thought when I first read the code. My interpretation ended up being that the sound card should not be "kicked" (interrupted) until delay more frames are added to the buffer. So, if you have latency 0 in the guest, queue 1024 bytes in the host sound card and return a latency of 1024 bytes to the guest right away, its runtime->delay value would be increased by 1024 bytes converted into frames. This way, it doesn't send all audio bytes to the card device as fast as possible.

I don't have strong doubts about this explanation to be honest but the best source for this would surely be the spec authors.

MatiasVara commented 10 months ago

https://github.com/rust-vmm/vhost-device/blob/573e592037d971058eda39d6f04ae2322d8f0701/staging/vhost-device-sound/src/lib.rs#L261

The doc comment can be similar to the commit that introduced the field:

sound: add latency_bytes field to IOMessage The latency field specifies how long it will take until the device finishes playing the I/O message's buffers.

Source:

https://lists.oasis-open.org/archives/virtio-dev/201912/msg00123.html

Do you think you could respond this also here https://lore.kernel.org/all/ZR6EzpQb6J1TxbM8@fedora/T/? I think that would be helpful for moving the discussion forward. Thanks.