Next 013

[HEnquist]

ping @pavhofman

[pavhofman]

Thanks!

That clippy - cargo clippy did not complain about the Eq derive. What is the correct check? Thanks.

[HEnquist]

Thanks!

That clippy - cargo clippy did not complain about the Eq derive. What is the correct check? Thanks.

I just ran cargo clippy with no other options. I used a quite recent version, will check exactly which one. EDIT it was added in 1.63, but I think was using the nightly from 1.61 when I got the warning.

[pavhofman]

Excellent, thanks a lot. Since the parameter align_bytes in calculate_aligned_period_near is basically optional (e.g. IntelHDAs have never more than 16 channels by specs), would it make sense to make it Option<u32>?

[HEnquist]

Sure it could be optional. Giving a 1 should do the same thing but having it optional is more intuitive. I'm a little confused about the math and what result this should give. Is this example correct?

rate = 48000
desired_period = 50 ms
framesize = 24 bits * 2 channels = 6 bytes
alignment = 128 bytes
alignment_bytes = lcm(6, 128) = 384 bytes
alignment_frames = 384 bytes / 6 bytes per frame = 64 frames
alignment_time = 64 / 48000 = 1.3333.. ms
n = round(desired_period / alignment_time) = round(37.5) = 38
aligned_period = 38*alignment_time = 50.666.. ms
aligned_period_ns00 = round(aligned_period * 1e7) = round(506666,666..) = 506667

[pavhofman]

IMO the calculation is correct. I wonder why the API designers opted for the period buffer size expressed in time units when the buffer requires precise byte alignment. Also the guessing game with the channel mask is quite annoying.

[HEnquist]

@pavhofman I updated the math now. Could you take a look?

[pavhofman]

Looks perfect, thanks a lot.

[HEnquist]

Alright, thanks! Then I think this is ready. I'll just do a little more testing before releasing it.

[pavhofman]

I like the demo you added. Maybe the part handling AUDCLNT_E_BUFFER_SIZE_NOT_ALIGNED could retry to get the aligned buffer as described in https://docs.microsoft.com/en-us/windows/win32/api/audioclient/nf-audioclient-iaudioclient-initialize#remarks - calling IAudioClient::GetBufferSize to get the closest properly aligned buffer size, convert it to 100ns using that MS formula, get a new audio client and retry initializing. It seems like getbuffersize is designed to return the closest aligned buffer size after AUDCLNT_E_BUFFER_SIZE_NOT_ALIGNED which is not obvious. While the current alignment code in the crate takes into account known requirements, it may not cover all possibilities, or some specific device can require some other alignment (which I assume its own getbuffersize implementation should respect).

Would it make sense to put that MS formula into a separate API helper method for cases like this?

Also, I could not move audioclient to the inner thread closure (I think it complained about lacking the Sync trait). But for exclusive mode it made sense to learn the buffer size in the outer thread and define the outer - inner channel with chunks of this size. So I had already audioclient in the outer thread which I would no longer use there, but could not move it to the inner thread. Instead I had to create another audioclient in the inner thread again. Please would you know what trait could be added to these structs to allow moving them between threads? Thanks a lot!

[HEnquist]

I will add a retry with the aligned buffer, I was already thinking about it, just haven't gotten around to doing it yet Having the formula as a separate function would make sense.

To pass an object between threads it must implement the Send trait. It's an automatically derived trait that you get if all parts of the object are also Send: https://doc.rust-lang.org/nomicon/send-and-sync.html The IAudioClient does not implement Send, it's actually marked as !Send which specifically prevents the Send trait. See https://microsoft.github.io/windows-docs-rs/doc/windows/Win32/Media/Audio/struct.IAudioClient.html I don't know if that !Send is strictly necessary. Maybe open an issue in windows-rs to ask?

[pavhofman]

Thanks a lot for your continuous improvements of the crate.

As of the missing Send trait -I did not realize it originates in the MS crate. I guess it has something to do with lots of unsafe code in the windows crate and discussion of !send in https://doc.rust-lang.org/nomicon/send-and-sync.html . Maybe it's there just for piece of mind of the devels :-) In the end it's nothing crucial, re-obtaining the client in the inner thread is no major slowdown.

[HEnquist]

I added a simple retry on unaligned buffers to the exclusive example. Nearly all of my devices seem to accept any value for the period, only my integrated Realtek ALC887 needs the alignment. I tested by modifying the example to intentionally set an unaligned period, which triggers the AUDCLNT_E_BUFFER_SIZE_NOT_ALIGNED error. After that the procedure from the docs recovers just fine and playback works. I wrote the recovery procedure as simple as possible, with no error handling etc.

I'll just update to the just released windows-rs v0.40.0, then I think this is ready.

[pavhofman]

Excellent addition, thanks!

Just a note - your Realtek ALC887 is a codec (DAC + ADC), hooked via HDA bus to IntelHDA controller integrated in the chipset. So your example is still for IntelHDA, codecs do not have any influence over the DMA transfer to the controller (which apparently requires the buffer alignment) https://learn.microsoft.com/en-us/windows-hardware/drivers/audio/images/hdaudio.png

[HEnquist]

Ok! I wasn't sure about that since it's an AMD machine, but I suppose it's still following the Intel HDA standard then. I'll update that in the docs.

Now I'm waiting for the docs for windows-rs to be updated to v0.40.0, there were some breaking changes I need to sort out.

[pavhofman]

Yes, IntelHDA is the standard on all x86/amd64 platforms (being AC97 successor). Google told me that recently AMD introduced one chipset (TRX40) with no integrated HDA controller, and MB vendors ended up using onboard USB audio-HDA bridge + HDA codecs (+ users facing driver problems, as usual) - this case would not be IntelHDA from this buffer alignment POW.