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

shared-module/audiomixer: expose MixerVoice.loop property #9861

Merged
merged 3 commits into from
Dec 5, 2024

Conversation

pdw-mb
Copy link

@pdw-mb pdw-mb commented Dec 4, 2024

This PR exposes MixerVoice.loop as a mutable Python property.

The motivation here is to make it possible to stop a looping sample at the end of its current loop. Without this, it is only possible to use stop() to stop the sample immediately.

My specific use case is a backing sound loop for a game, which then needs to transition into another sample. Without this feature, I can't use the loop option, as I can't stop the sample at the end of a loop. With this feature, I can set voice.loop = False, then poll voice.playing to detect the end of the loop and start the next sample. It'd be nice to able to queue up the next sample to remove the need to poll, but that's a bigger change.

Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

Thanks! This seems like a good idea. I have some comments, though.

Comment on lines 118 to 121
enum { ARG_loop };
static const mp_arg_t allowed_args[] = {
{ MP_QSTR_loop, MP_ARG_BOOL | MP_ARG_REQUIRED, {} },
};
Copy link
Member

Choose a reason for hiding this comment

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

for a property setter, there's no need to support kwargs. I notice that you were probably just copying the style of audiomixer_mixervoice_obj_set_level within this same file so what you wrote is understandable, but would be better to write it similar to this more usual setter:

static mp_obj_t synthio_synthesizer_obj_set_envelope(mp_obj_t self_in, mp_obj_t envelope) {
    synthio_synthesizer_obj_t *self = MP_OBJ_TO_PTR(self_in);
    check_for_deinit(self);
    synthio_synth_envelope_set(&self->synth, envelope);
    return mp_const_none;
}
MP_DEFINE_CONST_FUN_OBJ_2(synthio_synthesizer_set_envelope_obj, synthio_synthesizer_obj_set_envelope);

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I did wonder about that, and yes, very much a copy-and-paste job. Fixed in 372759f.

jepler added a commit to jepler/circuitpython that referenced this pull request Dec 4, 2024
@pdw-mb pdw-mb force-pushed the mixervoice-loop-property branch from c8e7930 to 372759f Compare December 4, 2024 19:42
@jepler
Copy link
Member

jepler commented Dec 4, 2024

Thanks! One particular board build is failing, but this is not an error in your change. Rather, the board's flash space is full. We'll have to do something to shrink that build before we can pull in this PR, so thank you for being patient about that.

Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

thank you!

@jepler jepler merged commit 8eb1103 into adafruit:main Dec 5, 2024
450 checks 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