fix(android): stop WaveformExtractor from killing the app, and decode PCM correctly - #2
fix(android): stop WaveformExtractor from killing the app, and decode PCM correctly#2victorrss wants to merge 4 commits into
Conversation
… PCM correctly Playing back a short clip crashed the process. The extractor sizes its buckets from the container's declared duration, but a decoder emits more frames than that implies — an AAC encoder delay alone is 2048 frames — so the buckets ran out before the stream did. On that overflow it called stop() from inside its own onOutputBufferAvailable callback, releasing the MediaCodec mid-buffer; the end-of-stream callback queued behind it then stopped the released codec and the app died on an IllegalStateException in MediaCodec.native_stop. Those trailing frames are padding, so drop them and let the stream drain to EOF instead, and make stop() idempotent for the races that remain (it is also reachable from stopExtraction on the platform thread). Completion was signalled by comparing progress to exactly 1.0F, which a clip ending early never reaches, leaving extractWaveformData pending forever. The extractor now owns its reply and answers on end-of-stream or on cancellation, so ExtractorCallBack has nothing left to do. Replies are posted to the main looper, where a MethodChannel.Result has to be answered. The samples themselves were wrong too. handle16bit widened the low byte of each little-endian sample without masking, so every sample over 0x7F had its high bits swallowed by the sign — roughly half of them were noise. handle8bit read unsigned PCM as signed, and the 32-bit path reinterpreted ENCODING_PCM_FLOAT data as integers. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the Android waveform extraction pipeline by preventing MediaCodec teardown races that can crash the app, ensuring extraction always completes (including early-ending clips/cancellation), and fixing PCM decoding correctness for multiple sample formats.
Changes:
- Makes
WaveformExtractor.stop()idempotent and moves final reply ownership into the extractor (posted to the main looper) instead of usingExtractorCallBack. - Prevents waveform bucket overflow from killing the app by dropping trailing padding frames and handling partial final buckets at EOF.
- Fixes PCM decoding for 8-bit unsigned, 16-bit little-endian sign/masking, and 32-bit float PCM.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| android/src/main/kotlin/com/simform/audio_waveforms/WaveformExtractor.kt | Fixes teardown race/crash, ensures completion reply, and corrects PCM decoding + bucket sizing/EOF handling. |
| android/src/main/kotlin/com/simform/audio_waveforms/AudioWaveformsPlugin.kt | Removes ExtractorCallBack usage and relies on extractor-owned completion. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } catch (e: Exception) { | ||
| inputEof = true | ||
| result.error( | ||
| Constants.LOG_TAG, | ||
| e.message, | ||
| "Invalid input buffer." | ||
| ) | ||
| submitError(e.message, "Invalid input buffer.") | ||
| } |
There was a problem hiding this comment.
Agreed, fixed in 4e9e4b3. No end-of-stream buffer was queued on that path, so the codec would never have reached EOF and would have kept running with its caller already answered. It now calls stop() right after submitError(); stop() is idempotent and nulls the decoder, so any callback still in flight returns early.
| totalSamples = (sampleRate.toLong() * durationMillis) / 1000 | ||
| perSamplePoints = totalSamples / expectedPoints | ||
| // A bucket of zero samples would divide by zero on | ||
| // every point when more points are asked for than the | ||
| // clip has samples. | ||
| perSamplePoints = max(1L, totalSamples / expectedPoints) |
There was a problem hiding this comment.
Right, this was a real crash path — integer division throws, and it would have thrown from the codec's callback thread. Fixed in 4e9e4b3 by flooring the count once into a pointCount field and using it everywhere a bucket is sized, including updateProgress(), which would otherwise have produced an infinite progress value.
| private fun sendProgress(rms: Float) { | ||
| sampleData.add(rms) | ||
| extractorCallBack.onProgress(progress) | ||
| sampleCount = 0 | ||
| sampleSum = 0.0 | ||
|
|
There was a problem hiding this comment.
Good catch, fixed in 4e9e4b3. Same defect class as the result reply this PR already moved to the main looper, so it should not have been left behind. It follows AudioRecorder.sendBytesToFlutter now, and posts a snapshot of the sample list rather than the live one, since the post outlives the callback and the list keeps growing until extraction ends.
… progress to the main looper Addresses review feedback: - A queueInputBuffer failure never queued an end-of-stream buffer, so the codec would keep running with its caller already answered. Tear down there. - expectedPoints arrives from Flutter and nothing stops it from being zero, which threw an ArithmeticException from onOutputFormatChanged. Floor it once and use that everywhere a bucket is sized. - invokeMethod ran on the codec's callback looper. Post it to the main looper, as AudioRecorder.sendBytesToFlutter already does, with a snapshot of the sample list since the post outlives the callback. Co-authored-by: Cursor <cursoragent@cursor.com>
…input buffer Stopping a recording could never finalize the file. End of stream is queued from onInputBufferAvailable, but the codec does not call back for a buffer it has already handed over, and by the time recording stops the input queue is drained with one such buffer held. Nothing then marked the end of the stream: the encoder never finished, stopEncoder never ran, the completion callback never fired, and the caller waited forever for a file left without its moov box. The leaked MediaMuxer only stopped later, from its finalizer. signalToStop now queues the end-of-stream buffer itself when it is holding one and the queue is drained. The end-of-stream timestamp calculation moves into a helper, shared with the callback path. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Closing: we are not taking the fork down this path for now. The branch stays available if we pick the extractor work back up. |
Description
Playing back a short clip crashes the app process on Android with an
IllegalStateExceptioninMediaCodec.native_stop, and the waveform it draws does not match the audio.WaveformExtractorsizes its buckets from the container's declared duration, but a decoder emits more frames than that implies — an AAC encoder delay alone is 2048 frames, plus up to one 1024-frame block of padding. Once those extra frames fill one more bucket, the extractor callsstop()from inside its ownonOutputBufferAvailablecallback, releasing the codec mid-buffer; the end-of-stream callback queued behind it then stops the released codec:A 2.3s clip at 16 kHz declares 36,864 frames but decodes 38,912, so at ~55 bars its ~670-frame buckets overflow on every playback. The example app never trips this: its 9.6s–105s assets at 44.1 kHz with the default 100 points leave 4,000+ frames per bucket. Recording a clip shorter than ~4.6s there reproduces it.
What this changes
stop()is idempotent for the races that remain — it is also reachable fromstopExtractionon the platform thread.1.0F. A clip that ends before it produced the requested number of points never reached that value, leavingextractWaveformDatapending forever.ExtractorCallBackexisted only to carry that signal and is removed. Replies are posted to the main looper, where aMethodChannel.Resulthas to be answered.handle16bitwidened the low byte of each little-endian sample without masking, so every sample over0x7Fhad its high bits swallowed by the sign — roughly half of them were noise.handle8bitread unsigned PCM as signed, and the 32-bit path reinterpretedENCODING_PCM_FLOATdata as integers.perSamplePointshas a floor of 1, so asking for more points than the clip has samples no longer divides by zero.iOS is unaffected:
WaveformExtractor.swiftiterates exactlysamplesPerPixelbuckets and calls its completion unconditionally.Checklist
fix:,feat:,docs:etc).docsand added dartdoc comments with///. The behaviour is internal to the Android extractor and the reasoning is documented in the source comments; no public API is described differently.examplesordocs. No example change: this is a bug fix behind the existing API.Breaking Change?
The Dart API is untouched. The removed
ExtractorCallBackinterface and the changedWaveformExtractorconstructor are Android-internal to the plugin and are not reachable from Dart.