Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drive audio directly using cpal, position sounds precisely within callbacks #36

Merged
merged 4 commits into from
Apr 28, 2023

Conversation

strohel
Copy link
Member

@strohel strohel commented Apr 26, 2023

This is a form of alternative to #35 to resolve #19 (comment)

Note that this should resolve the specific problem, but we still may want to use fundsp (somehow) to synthetize and modify sound.

We still use rodio for audio file decoding and processing, but we drive the cpal audio callback directly (using code adapted from rodio itself).

That allows us to run custom code during the callback, which we use to offset samples tasked to play (giving all samples the same delay of one period, rather than variable delay based on how close next to the next callback they fire).

See example print of ns_since_last_callback when we play 1000 samples a second The period length is apparently ~42 ms.
ns_since_last_callback:  38695172.
ns_since_last_callback:  39784768.
ns_since_last_callback:  40887270.
ns_since_last_callback:  41945052.
ns_since_last_callback:    368139.
ns_since_last_callback:   1477218.
ns_since_last_callback:   2445557.
ns_since_last_callback:   3573154.
ns_since_last_callback:   4616437.
ns_since_last_callback:   5675630.
ns_since_last_callback:   6764239.
ns_since_last_callback:   7835255.
ns_since_last_callback:   8925809.
ns_since_last_callback:   9995435.
ns_since_last_callback:  11159180.
ns_since_last_callback:  12234090.
ns_since_last_callback:  13312871.
ns_since_last_callback:  14392252.
ns_since_last_callback:  15460542.
ns_since_last_callback:  16545115.
ns_since_last_callback:  17564547.
ns_since_last_callback:  18582770.
ns_since_last_callback:  19640031.
ns_since_last_callback:  20683684.
ns_since_last_callback:  21803771.
ns_since_last_callback:  22879984.
ns_since_last_callback:  23956644.
ns_since_last_callback:  25022856.
ns_since_last_callback:  26101435.
ns_since_last_callback:  27179575.
ns_since_last_callback:  28249203.
ns_since_last_callback:  29342603.
ns_since_last_callback:  30412447.
ns_since_last_callback:  31495496.
ns_since_last_callback:  32519447.
ns_since_last_callback:  33580415.
ns_since_last_callback:  34606142.
ns_since_last_callback:  35697450.
ns_since_last_callback:  36791844.
ns_since_last_callback:  37869512.
ns_since_last_callback:  38943775.
ns_since_last_callback:  40022488.
ns_since_last_callback:  41100993.
ns_since_last_callback:  42180112.
ns_since_last_callback:    481551.
ns_since_last_callback:   1608677.
ns_since_last_callback:   2667820.
ns_since_last_callback:   3745600.
ns_since_last_callback:   4785690.

With this I'm able to go from tick_probe 1 all the way to tick_probe 4000 (!) and still hear difference. [1]

[1] However, there seem to be inefficiencies somewhere that result in e.g. composer saying Received 860 events (3440 bytes) in last 1.00s. when we call tick_probe 1000. May be naive timing code in tick_probe itself (@PabloMansanet?).

@strohel strohel requested review from bschwind and goodhoko April 26, 2023 21:27
…lbacks

This is a form of alternative to #35 to resolve #19 (comment)

Note that this should resolve the specific problem, but we still may want to use `fundsp` (somehow) to synthetize and modify sound.

We still use `rodio` for audio file decoding and processing, but we drive the `cpal` audio callback directly (using code adapted from `rodio` itself).

That allows us to run custom code during the callback, which we use to offset samples tasked to play (giving all samples the same delay of one period, rather than variable delay based on how close next to the next callback they fire).

<details><summary>See example print of ns_since_last_callback when we play 1000 samples a second</summary>
The period length is apparently ~42 ms.

```
ns_since_last_callback:  38695172.
ns_since_last_callback:  39784768.
ns_since_last_callback:  40887270.
ns_since_last_callback:  41945052.
ns_since_last_callback:    368139.
ns_since_last_callback:   1477218.
ns_since_last_callback:   2445557.
ns_since_last_callback:   3573154.
ns_since_last_callback:   4616437.
ns_since_last_callback:   5675630.
ns_since_last_callback:   6764239.
ns_since_last_callback:   7835255.
ns_since_last_callback:   8925809.
ns_since_last_callback:   9995435.
ns_since_last_callback:  11159180.
ns_since_last_callback:  12234090.
ns_since_last_callback:  13312871.
ns_since_last_callback:  14392252.
ns_since_last_callback:  15460542.
ns_since_last_callback:  16545115.
ns_since_last_callback:  17564547.
ns_since_last_callback:  18582770.
ns_since_last_callback:  19640031.
ns_since_last_callback:  20683684.
ns_since_last_callback:  21803771.
ns_since_last_callback:  22879984.
ns_since_last_callback:  23956644.
ns_since_last_callback:  25022856.
ns_since_last_callback:  26101435.
ns_since_last_callback:  27179575.
ns_since_last_callback:  28249203.
ns_since_last_callback:  29342603.
ns_since_last_callback:  30412447.
ns_since_last_callback:  31495496.
ns_since_last_callback:  32519447.
ns_since_last_callback:  33580415.
ns_since_last_callback:  34606142.
ns_since_last_callback:  35697450.
ns_since_last_callback:  36791844.
ns_since_last_callback:  37869512.
ns_since_last_callback:  38943775.
ns_since_last_callback:  40022488.
ns_since_last_callback:  41100993.
ns_since_last_callback:  42180112.
ns_since_last_callback:    481551.
ns_since_last_callback:   1608677.
ns_since_last_callback:   2667820.
ns_since_last_callback:   3745600.
ns_since_last_callback:   4785690.
```

</details>

With this I'm able to got from `tick_probe 1` all the way to `tick_probe 4000` (!) and still hear difference. [1]

[1] However, there seem to be inefficiencies somewhere that result in e.g. `composer` saying `Received 860 events (3440 bytes) in last 1.00s.` when we call `tick_probe 1000`. May be naive timing code in `tick_probe` itself (@PabloMansanet?).
@PabloMansanet
Copy link
Contributor

[1] However, there seem to be inefficiencies somewhere that result in e.g. composer saying Received 860 events (3440 bytes) in last 1.00s. when we call tick_probe 1000. May be naive timing code in tick_probe itself (@PabloMansanet?).

I see that you guys pushed a change to try and fix this, did it work? :)

@goodhoko
Copy link
Member

@PabloMansanet

I see that you guys pushed a change to try and fix this, did it work? :)

It did, now the test probe is almost perfectly accurate in maintaining the given frequency. .)

@PabloMansanet
Copy link
Contributor

Awesome! I'll give this a more in-depth review after lunch then :)

Copy link
Contributor

@PabloMansanet PabloMansanet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I personally find this code easier to follow than #35, (mostly because it doesn't need the powerful generics).

strohel referenced this pull request Apr 27, 2023
This way is slightly more elaborate, but should be as precise as it gets - it uses playback timestamp from the audio system itself.
Copy link
Member

@goodhoko goodhoko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting serious. .) I like how you were able to get to the low-level precise timing while still using the rodio's APIs.

Just a few minor suggestions (one of them as separate PR)

crates/composer/src/sound.rs Outdated Show resolved Hide resolved
crates/composer/src/sound.rs Outdated Show resolved Hide resolved
Comment on lines 112 to 126
let stream_timestamp = info.timestamp();
let stream_start =
self.stream_start.get_or_insert_with(|| StreamStart::new(stream_timestamp));

// At least on Linux ALSA, cpal gives very strange stream timestamp on very first call.
if stream_start.callback > stream_timestamp.callback {
eprintln!("cpal's stream timestamp jumped backwards, resetting stream start.");
*stream_start = StreamStart::new(stream_timestamp);
}

// UNIX timestamp of when the buffer filled during this call will be actually played.
let buffer_playback_timestamp = stream_start.realtime
+ (stream_timestamp.playback.duration_since(&stream_start.callback).expect(
"current playback timestamp should be larger than start callback timestamp",
));
Copy link
Member

@goodhoko goodhoko Apr 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had hardtime understanding this timstamp-jutsu but after some sketches in a notebook it clicked and I was then able to simplify it a bit in #39

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, merged in. @goodhoko I'd be curious to see your sketches!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hehe, I don't think it will be comprehensible though

IMG_7712

the "ball" is the sound we're trying to position, the line on right is the destination buffer with the question mark being the delay we need to compute. On the left there are the two independent epochs. The arrows are... well I guess just my thoughts as I was figuring it out .D

goodhoko and others added 2 commits April 28, 2023 13:53
Instead of caching the stream_start compute the offset for every
callback invocation.
@strohel
Copy link
Member Author

strohel commented Apr 28, 2023

@goodhoko addressed your comments in the last commit, please feel free to merge this while I'm babysitting.

@goodhoko goodhoko merged commit 70df76e into main Apr 28, 2023
@goodhoko goodhoko deleted the direct-cpal branch April 28, 2023 12:33
@strohel strohel mentioned this pull request Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add event timestamping
3 participants