Repository navigation
feat: add MP3 streaming encoder (item 8's MP3 sub-item) - #120
Merged
Merged
Conversation
Implements the MP3 sub-item of docs/streaming-plan.md item 8: Mp3StreamingEncoder is the third IStreamingEncoder implementation, reusing Mp3FrameEncoder's own encode internals unchanged -- only how each encoded frame is delivered differs (a callback emitting raw, self-delimiting MPEG frames instead of writing bytes to a file). Writes no dedicated "Info tag" (LAME-tag) header frame and no ID3v2 tag at all -- both are container/file-level conventions with no equivalent for a raw, open-ended frame stream, the same reasoning that already excluded ADTS framing from AacStreamingEncoder's own raw access units and Ogg/pre_skip from OpusStreamingEncoder's own raw packets. Mp3Encoder.EncoderDelaySamples (the existing, already empirically-measured 1680) remains the right value for a muxer here to use directly. Unlike OpusStreamingEncoder, this encoder reports each frame's own real sample count rather than always the nominal 1152 -- tracking it costs nothing extra here, the same way it doesn't for AacStreamingEncoder, since Mp3EncoderSession.Finish's own existing design already computes the real remaining count before padding it. Complete()'s own mandatory flush frame (draining the one-granule MDCT history, the same real, previously-found-and-fixed requirement Mp3EncoderSession.Finish itself documents) is purely synthetic and reports 0. FLAC streaming sessions (item 8's remaining sub-item) remain not started. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Second self-review pass: both call sites always passed the same FrameSamples constant, so the parameter added indirection with no actual flexibility. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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
docs/streaming-plan.mditem 8:Mp3StreamingEncoderis the thirdIStreamingEncoderimplementation, reusingMp3FrameEncoder's own encode internals unchanged — only how each encoded frame is delivered differs (a callback emitting raw, self-delimiting MPEG frames instead of writing bytes to a file).AacStreamingEncoder's own raw access units (PR feat: add push-style IStreamingEncoder + AAC-LC streaming session #110) and Ogg/pre_skipfromOpusStreamingEncoder's own raw packets (PR feat: add Opus streaming encoder (item 8's Opus sub-item) #118).Mp3Encoder.EncoderDelaySamples(the existing, already empirically-measured1680) remains the right value for a muxer here to use directly.OpusStreamingEncoder, this encoder reports each frame's own real sample count rather than always the nominal1152— tracking it costs nothing extra here, the same way it doesn't forAacStreamingEncoder, sinceMp3EncoderSession.Finish's own existing design already computes the real remaining count before padding it.Complete()'s own mandatory flush frame (draining the one-granule MDCT history — the same real, previously-found-and-fixed requirementMp3EncoderSession.Finishitself documents) is purely synthetic and reports a real sample count of0.docs/streaming-plan.md(MP3 sub-item marked done),CLAUDE.md,README.md,llms.txt,llms-full.txt.Test plan
Mp3StreamingEncoderTest: construction validation (null callback, unsupported channels/sample rate/bitrate),SamplesPerFrame == 1152,Complete()with zero samples emits nothing, idempotentComplete()/safe doubleDispose(),WriteafterComplete()throws, monotonic frame indices, exact per-frameSampleCountverification (real count for the padded tail frame,0for the synthetic flush frame — including the "exactly one full frame, nothing pending" edge case), real round-trip SNR for mono and stereo (concatenating raw frames into a file, decoding via the project's ownMp3Decoder/NLayer), and a regression test mirroringMp3EncoderSnrTest's own exact-tail-preservation check, confirming this restructured callback-based version of that fix still holds.dotnet test, net10.0 locally): 1703 passed, 1 pre-existing unrelated skip, 0 failed.🤖 Generated with Claude Code