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

external hook #122

Merged
merged 2 commits into from
Jun 8, 2024
Merged

external hook #122

merged 2 commits into from
Jun 8, 2024

Conversation

bwhitman
Copy link
Collaborator

@bwhitman bwhitman commented Jun 8, 2024

No description provided.

@dpwe
Copy link
Collaborator

dpwe commented Jun 8, 2024

Looks good.

  • I would pass num_samples to the external hook. Of course it's always AMY_BLOCK_SIZE, but this external hook may be ignorant about Amy internals. It just makes sense as a signature IMO.
  • SAMPLES is not the greatest type to be exporting - it's Amy-internal, and somewhat nonstandard (sign bit, 8 integer bits, 23 fractional bits). As it stands, the external hook is going to have to be pretty well-informed about Amy internals. I guess that's OK. The alternative would be to convert to int16 (like the waveform output) or float, but I can see why you don't want to do that.
  • Why not have the function return a bool saying whether it consumed the samples? Less clumsy than a pointer to a return value.

@bwhitman
Copy link
Collaborator Author

bwhitman commented Jun 8, 2024

OK on (1). on (2) i agree but not sure i want to do all that work per block. (3) sure, but i am not sure how to set up a function pointer that returns a value. i can ask the internet

@bwhitman
Copy link
Collaborator Author

bwhitman commented Jun 8, 2024

Fixed up for (1) and (3). Gonna merge

@bwhitman bwhitman merged commit b783fd4 into main Jun 8, 2024
1 check passed
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.

2 participants