Skip to content

RSDK-13419 Add stop playing audio do command#27

Merged
oliviamiller merged 2 commits intoviam-modules:mainfrom
oliviamiller:stop-play
Feb 23, 2026
Merged

RSDK-13419 Add stop playing audio do command#27
oliviamiller merged 2 commits intoviam-modules:mainfrom
oliviamiller:stop-play

Conversation

@oliviamiller
Copy link
Collaborator

@oliviamiller oliviamiller commented Feb 17, 2026

Summary
Adds a do command to stop playing audio from the speaker. (requested by gambit).

Testing
Added unit test for successful stop, manually tested on mac and linux that audio stopped playing.

Reviews
1 Approval.


The speaker supports the following DoCommands:

**`set_volume`** — Set the speaker output volume.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

forgot to document this before so adding now

Copy link

@SebastianMunozP SebastianMunozP left a comment

Choose a reason for hiding this comment

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

LGTM

// Advance playback position to write position so no more audio is played.
std::lock_guard<std::mutex> lock(stream_mu_);
if (audio_context_) {
audio_context_->playback_position.store(audio_context_->get_write_position(), std::memory_order_relaxed);
Copy link

@dgottlieb dgottlieb Feb 23, 2026

Choose a reason for hiding this comment

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

I don't think using memory_order_relaxed is correct here. Unless we truly don't care what the value is. If the reader of playback_position also happened to acquire the stream_mu_, this could be accidentally right (using a mutex + atomic variables is awkward, I don't believe we're intentionally trying to rely on some memory ordering relationship there).

In general, one shouldn't need to play with the default memory_order_seq_cst that these stores and loads use.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oops sorry I merged before I saw this, ill make a ticket to remove the memory_order usages and verify there's no performance regression in the audio callback.

@oliviamiller oliviamiller merged commit e7cd07b into viam-modules:main Feb 23, 2026
5 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.

3 participants