Mic: never write past the record() buffer; reject empty requests - #370
Merged
Merged
Conversation
The capture task emitted a whole conversion step per iteration (2 samples, or 4 for mono in -> stereo out) regardless of how many elements the destination had left, so any array_len that was not a multiple of the step overran the buffer by up to 3 elements (measured on a mono-mic device: 2 bytes for odd 16-bit mono, 6 bytes for mono -> stereo). Emit per time step instead, and when a mono capture step straddles two buffers keep the second time step for the next queued request, so back-to-back recordings stay continuous whatever their length. The carry is dropped only when no request is pending (the buffered input is dropped there as well). A stereo buffer whose last element is a left sample ends with left only, which is now documented. record() with array_len == 0 (or a null pointer) used to start the mic and return true without queueing anything, so the release callback never fired for a "successful" request. It now returns false.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mic.record()could write past the end of the caller's buffer, andrecord()witharray_len == 0reported success without recording anything.The capture task emitted a whole conversion step per iteration (2 samples, or 4 when a mono microphone is recorded into a stereo buffer) without checking how many elements the destination had left. Any
array_lenthat was not a multiple of the step overran the buffer by up to 3 elements. Measured on a mono-microphone device with a guard pattern after the buffer: odd-length 16-bit mono overran by 2 bytes, mono into stereo by up to 6 bytes.Changes
Mic_Class.inl: the task writes per time step. When a mono capture step straddles two buffers, the second time step is kept for the next queued request, so back-to-back recordings stay continuous whatever their length. The carry is dropped only when no request is pending (the buffered input is dropped there as well). A stereo buffer whose last element is a left sample ends with left only._rec_raw():array_len == 0or a null pointer returnsfalse. Previously it started the microphone and returnedtruewithout queueing anything, so the release callback never fired for a "successful" request.Mic_Class.hpp: therecord()contract is documented (exact element count, odd stereo length, continuity of queued requests, invalid arguments).Public signatures are unchanged. All
record()calls in the examples use a non-zero length.Verification
Mono-microphone device (ESP32), guard bytes on both sides of the destination, 17 cases covering 8/16-bit, mono/stereo, lengths 0, 1, 3, 253, 255, 256, 257, 258, 4000, 4001:
record(…, 0)returned truerecord(…, 0)returns falseTwo rounds of independent adversarial review (the first found that clamping alone dropped samples at buffer boundaries, which led to the carry-over). CI run on the ainyan03 fork before this PR.