[rtttl] Fix speaker playback bugs by swoboda1337 · Pull Request #14280 · esphome/esphome

swoboda1337 · 2026-02-25T17:09:36Z

What does this implement/fix?

Fixes multiple bugs in the RTTTL speaker playback path:

finish_() buffer overread: passed 8 bytes to speaker_->play() but sample array is only sizeof(SpeakerSample) * 2 = 4 bytes, reading past the array (UB). Fixed to use sizeof(sample).
play() size mismatch: play() was called with x * 2 bytes but the completion check compared against x * 4. Fixed both to use x * sizeof(SpeakerSample).
Note timing divisor: samples_count_ and samples_gap_ were calculated with /1600 instead of /1000, causing notes to play at 62.5% of their intended duration. sample_rate_ is in samples/sec and note_duration_ is in milliseconds, so dividing by 1000 converts ms to seconds:
```
samples = (samples/sec × ms) / 1000 = (samples/sec × sec) = samples
```
With /1600, a 300ms note at 16kHz produces 3000 samples (187.5ms) instead of the correct 4800 samples (300ms).
NOTES array bounds check: used sizeof(NOTES) (98 bytes for 49 uint16_t elements) instead of element count. Fixed to sizeof(NOTES) / sizeof(NOTES[0]).
get_integer_() overflow: returned uint8_t, so BPM values > 255 would silently wrap. RTTTL spec allows BPM up to 900. Changed to uint16_t.
Uninitialized output_ pointer: output::FloatOutput *output_ was not initialized, could cause UB if USE_OUTPUT is defined but no output is set. Initialized to nullptr.
Note gap underflow guard: the output path subtracts DOUBLE_NOTE_GAP_MS from note_duration_ without checking the duration is large enough. With the uint16_t fix (Remove sleeps. #5) allowing higher BPMs and short note durations (e.g. 32nd note at BPM 900 = 8.3ms < 10ms gap), this could underflow the uint16_t. Added a guard to skip the gap when the note is too short.

Testing

Tested on Waveshare ESP32-P4 Nano with ES8311 codec and I2S speaker output.

Mario theme (mario:d=4,o=5,b=100:16e6,16e6,32p,8e6,...):

Calculated duration: 6.9s (30 notes at BPM 100)
Before fix (/1600): 4.72s — notes played at 62.5% of intended duration
After fix (/1000): 7.53s — correct duration (6.9s + speaker start/stop overhead)

Before:
[12:03:52.518][D][rtttl:293]: Playing song mario
[12:03:52.518][D][i2s_audio.speaker:102]: Starting
[12:03:57.240][D][i2s_audio.speaker:111]: Stopping
[12:03:57.245][D][rtttl:419]: Playback finished
Duration: ~4.72s

After:
[12:07:59.465][D][rtttl:293]: Playing song mario
[12:07:59.466][D][i2s_audio.speaker:102]: Starting
[12:08:07.005][D][i2s_audio.speaker:111]: Stopping
[12:08:07.007][D][rtttl:420]: Playback finished
Duration: ~7.53s

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Developer breaking change (an API change that could break external components)
Code quality improvements to existing code or addition of tests
Other

Related issue or feature (if applicable):

N/A

Pull request in esphome-docs with documentation (if applicable):

N/A

Test Environment

Example entry for `config.yaml`:

i2s_audio:
  - id: i2s_output
    i2s_lrclk_pin: GPIO10
    i2s_bclk_pin: GPIO12
    i2s_mclk_pin: GPIO13

speaker:
  - platform: i2s_audio
    i2s_audio_id: i2s_output
    id: speaker_id
    i2s_dout_pin: GPIO09
    dac_type: external
    sample_rate: 16000
    bits_per_sample: 16bit
    channel: mono

rtttl:
  speaker: speaker_id

Checklist:

The code change is tested and works locally.
Tests have been added to verify that the new code works (under tests/ folder).

- Fix finish_() reading past SpeakerSample array (8 bytes for 4 byte buffer) - Fix play() size mismatch (compared against x*4 instead of x*sizeof(SpeakerSample)) - Fix note duration divisor (1600 should be 1000, notes played 37.5% too short) - Fix NOTES array bounds check using sizeof bytes instead of element count - Fix get_integer_() overflow for BPM > 255 (RTTTL spec allows up to 900) - Initialize output_ pointer to nullptr Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-02-25T17:09:58Z

To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file:

external_components:
  - source: github://pr#14280
    components: [rtttl]
    refresh: 1h

(Added by the PR bot)

github-actions · 2026-02-25T17:09:59Z

👋 Hi there! I've automatically requested reviews from codeowners based on the files changed in this PR.

@glmnet - You've been requested to review this PR as codeowner(s) of 2 file(s) that were modified. Thanks for your time! 🙏

Copilot

Pull request overview

This PR fixes multiple critical bugs in the RTTTL speaker playback path that caused buffer overruns, timing errors, and potential undefined behavior. The changes address fundamental issues with buffer size calculations, note timing, array bounds checking, and integer overflow handling.

Changes:

Fixed buffer size calculations in speaker playback code to use sizeof() correctly
Corrected note timing divisor from /1600 to /1000 for accurate tempo
Fixed NOTES array bounds check to use element count instead of byte size
Changed get_integer_() return type to uint16_t to support BPM values > 255
Initialized output_ pointer to nullptr to prevent undefined behavior

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
esphome/components/rtttl/rtttl.h	Changed `get_integer_()` return type to uint16_t and initialized `output_` pointer to nullptr
esphome/components/rtttl/rtttl.cpp	Fixed buffer size calculations, note timing divisor, NOTES array bounds check, and BPM variable type

codecov-commenter · 2026-02-25T17:12:34Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.28%. Comparing base (37a0cec) to head (32f5ae1).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files

@@           Coverage Diff           @@
##              dev   #14280   +/-   ##
=======================================
  Coverage   74.28%   74.28%           
=======================================
  Files          55       55           
  Lines       11595    11595           
  Branches     1583     1583           
=======================================
  Hits         8613     8613           
  Misses       2578     2578           
  Partials      404      404

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2026-02-25T17:15:58Z

Memory Impact Analysis

Components: rtttl
Platform: esp8266-ard

Metric	Target Branch	This PR	Change
RAM	29,240 bytes	29,240 bytes	➡️ +0 bytes (0.00%)
Flash	283,467 bytes	283,591 bytes	📈 🔸 +124 bytes (+0.04%)

📊 Component Memory Breakdown

Component	Target Flash	PR Flash	Change
`[esphome]rtttl`	2,151 bytes	2,255 bytes	📈 🚨 +104 bytes (+4.83%)
`app_framework`	2,049 bytes	2,053 bytes	📈 +4 bytes (+0.20%)

🔍 Symbol-Level Changes (click to expand)

Changed Symbols

Symbol	Target Size	PR Size	Change
`esphome::rtttl::Rtttl::loop()`	458 bytes	502 bytes	📈 +44 bytes (+9.61%)
`setup`	674 bytes	678 bytes	📈 +4 bytes (+0.59%)

Note: This analysis measures static RAM and Flash usage only (compile-time allocation).
Dynamic memory (heap) cannot be measured automatically.
⚠️ You must test this PR on a real device to measure free heap and ensure no runtime memory issues.

This analysis runs automatically when components change. Memory usage is measured from a representative test configuration.

Skip the inter-note gap when the note duration is too short to accommodate it, preventing uint16_t underflow on note_duration_. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

ximex

what i understood, looks good

bdraco · 2026-02-25T19:25:05Z

esphome/components/rtttl/rtttl.cpp

-    this->samples_count_ = (this->sample_rate_ * this->note_duration_) / 1600;  //(ms);
+    this->samples_count_ = (this->sample_rate_ * this->note_duration_) / 1000;
    if (need_note_gap) {
-      this->samples_gap_ = (this->sample_rate_ * DOUBLE_NOTE_GAP_MS) / 1600;  //(ms);
+      this->samples_gap_ = (this->sample_rate_ * DOUBLE_NOTE_GAP_MS) / 1000;


Trying to figure out why this was 1600 before

samples per sec * note duration / 1000 = samples..

but why was it 1600

I guess someone assumed the sample rate was always 16000 ?

but we are converting ms to s here so 1000 seems right

Yeah I dont get it either. Need to divide by 1000 to convert to ms, not sure where 1600 came from

AFAICT its just always been wrong even in #5177

swoboda1337 · 2026-02-25T19:34:41Z

Thanks

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

swoboda1337 requested a review from glmnet as a code owner February 25, 2026 17:09

Copilot AI review requested due to automatic review settings February 25, 2026 17:09

esphome bot added bugfix component: rtttl small-pr PR < 30 lines labels Feb 25, 2026

Copilot started reviewing on behalf of swoboda1337 February 25, 2026 17:10 View session

swoboda1337 marked this pull request as draft February 25, 2026 17:12

Copilot AI reviewed Feb 25, 2026

View reviewed changes

swoboda1337 mentioned this pull request Feb 25, 2026

[rtttl] improve comments Part 2 #13971

Merged

17 tasks

swoboda1337 marked this pull request as ready for review February 25, 2026 17:14

swoboda1337 added this to the 2026.2.2 milestone Feb 25, 2026

[rtttl] Guard note gap subtraction against underflow

32f5ae1

Skip the inter-note gap when the note duration is too short to accommodate it, preventing uint16_t underflow on note_duration_. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

ximex approved these changes Feb 25, 2026

View reviewed changes

bdraco reviewed Feb 25, 2026

View reviewed changes

bdraco approved these changes Feb 25, 2026

View reviewed changes

swoboda1337 merged commit d1a636a into esphome:dev Feb 25, 2026
28 checks passed

jesserockz pushed a commit that referenced this pull request Feb 26, 2026

[rtttl] Fix speaker playback bugs (#14280)

a39be5a

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jesserockz mentioned this pull request Feb 26, 2026

2026.2.2 #14303

Merged

jesserockz added the cherry-picked label Feb 26, 2026

github-actions bot locked and limited conversation to collaborators Feb 27, 2026

Uh oh!

Conversation

swoboda1337 commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this implement/fix?

Testing

Types of changes

Test Environment

Example entry for config.yaml:

Checklist:

Uh oh!

github-actions bot commented Feb 25, 2026

Uh oh!

github-actions bot commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

codecov-commenter commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Memory Impact Analysis

Changed Symbols

Uh oh!

ximex left a comment

Choose a reason for hiding this comment

Uh oh!

bdraco Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

bdraco Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

bdraco Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

bdraco Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

swoboda1337 Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

bdraco Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

swoboda1337 commented Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

swoboda1337 commented Feb 25, 2026 •

edited

Loading

Example entry for `config.yaml`:

codecov-commenter commented Feb 25, 2026 •

edited

Loading

github-actions bot commented Feb 25, 2026 •

edited

Loading