Commit Graph
1 Commits
Author SHA1 Message Date
Aleksander HelgakerandClaude Opus 5.5 ca99f91a18 craft-audio: AAC encoder (6/10) (#444)
* craft-audio: AAC decoder

FilmCraft's `filmcraft-aac` decoder as `craft_audio::aac` behind
`aac-decode`, kept over symphonia's: the same quality (about 135 dB
SNR against ffmpeg), and it decodes 5.1, which symphonia refuses.
HE-AAC decodes as its AAC-LC core, upsampled 2x to the stream's rate.

- `TrackDecoder` decodes AAC, including ADTS-framed packets without
  extradata (MPEG-TS, raw `.aac`).
- The native read path takes AAC in MP4 and Matroska and raw ADTS
  streams (frame sync over the file, with resync after damage).
- AAC errors convert into `AudioError`.

The AAC module's shared parts (Huffman, MDCT, TNS, ICS) arrive whole
as ported; their encoder half is unused until the encoder PR.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 1 (#443)

Raw ADTS and TrackDecoder:
- Channel configuration 0 (a PCE in the stream, as ffmpeg's -aac_pce and
  AudioToolbox's 7.x write) decodes; it was refused.
- Frames with several raw data blocks are refused; only their first block
  was decoded, silently losing the rest.
- A frame that changes the sample rate, profile or channel configuration is
  concealed; it played at the wrong speed.
- A source that fails during the frame scan is AudioError::Io; it looked
  like the end of the file.
- A stream after an ID3 tag longer than the file head is found.
- conceal counts all the ADTS frames of a packet, and a packet of more than
  1024 frames is refused before it is decoded into memory.

AAC decoder:
- Indexed channel configurations place elements by arrival order, not tag,
  as ffmpeg does; tagged differently, channels were swapped.
- A PCE in an access unit that fails no longer changes the layout, which
  left every later packet failing.
- Non-finite samples are zeroed before the output and the overlap.

MP4: probe and decode agree: the samples' durations end the track when no
single edit does, and an edit cannot outlast the media.

The routing oracle fails on a fixture ffmpeg cannot write when oracles are
required. The README's Limits names HE-AAC v2 as mono, multi-block ADTS,
iTunSMPB, Matroska's tick precision and AAC in CAF.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 2 (#443)

Corrects round 1:
- A multi-block ADTS frame after the stream has started is concealed at its
  whole length; round 1's filter skipped it, shifting the audio after it.
- Sample durations only trim an MP4's last packet: wrong ones (some count a
  HE-AAC packet's core frames) were cutting audio. Durations of 0 with an edit
  use the edit.

Raw ADTS packets now keep their headers, so TrackDecoder alone handles format
changes, multi-block frames and concealment, for apps as for files, and nested
ADTS payloads are not split again. Headers come from the frame scan's reads,
not one read per frame. The stream's format is the one most of its first frames
share, so one damaged header does not silence the file. The ID3 walk and a
packet's ADTS split are bounded.

AAC:
- HE-AAC is delayed by the SBR filterbanks' 481 core samples; it played 19.5 ms
  early against ffmpeg in every container.
- A failed access unit leaves the decoder's state as it was, so the next packet
  decodes as if it had not been there.
- reset restarts the noise generator, so a seek into PNS repeats.
- A signalled rate that is not twice the core's plays the core; it reported the
  signalled rate with the core's samples.

The decoder oracle's tolerance tightens from 1e-3 to 1e-5. Tests cover HE-AAC.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 3 (#443)

Corrects round 2:
- ADTS headers are compared with the stream's format only when the first header
  gave it. With extradata the format comes from there, so ADTS in MP4 whose
  headers say something else decodes again; every packet had been concealed.
- A stream's only program config element is found in its first frames even when
  that frame's header is damaged.
- The test of wrong sample durations patched the second run's count, not the
  delta, so "durations of one" changed nothing. It now patches the delta, and
  checks a file was changed and that seeks land on the same audio.

MP4 seeks place a packet by its index when every packet decodes to the same
length, as decoding does; they used the durations round 2 had stopped trusting
for the length, and returned the wrong audio. A last delta of 0 is overridden too.

Docs: what TrackInfo says for raw ADTS, that opening one reads the whole file, and
which codecs symphonia reads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 4 (#443)

- TrackDecoder takes an ADTS stream's format from the one most of the lookahead
  shares, as the file path did, so one damaged first header does not fail every
  packet of an MPEG-TS stream. open_adts now just asks it.
- MP4 samples of several ADTS frames keep the sample table's timing; a packet
  count of one frame each truncated them silently.
- Sample durations past what the packets decode to no longer over-report the
  length.
- split_adts_upto is not behind aac-decode, as split_adts is not: an aac-encode
  build in the next PR did not compile.

Removes an unreachable ID_END check and open_adts's unused core rate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 5 (#443)

No change in decoding. An aac-encode build in the next PR needs the
split_adts_upto re-export behind aac-decode, and ID_END only in tests.

Docs say the lookahead's headers give an ADTS stream's configuration, that AAC in
CAF still goes through symphonia, and that ProgramConfig is not #[non_exhaustive].

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 6 (#443)

Two tests from round 1 failed on ffmpeg before 9, including CI's 8.0. Neither was
a decoding fault:
- ffmpeg 9 ends an MP4 with no edit list at its sample durations and earlier
  versions present the whole last packet, so the length is checked against the
  file's known duration, not ffmpeg's decode.
- The order ffmpeg gives channels with -aac_pce differs by version, so each decoded
  channel must match a different one of ffmpeg's, whatever the order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 7 (#443)

Reading:
- an element of another type or a tag a program config does not list is
  refused, not written to another channel; only an LFE coded as an SCE
  is still allowed, as ffmpeg allows it
- an explicit sampling rate of 0 is refused
- a packet's later ADTS frames are searched for SBR
- a multi-frame ADTS packet that fails leaves the decoder as it was, and
  one whose frames change the channel count is an error
- the whole ADTS frames before a damaged end of a packet decode
- a failed read of a raw stream's first frames is an I/O error
- an MP4 sample of several ADTS frames past the lookahead keeps its length
- an Ogg file with leading bytes keeps its track order and mask comments

Writing:
- ProgramConfig::write, AudioSpecificConfig::to_bytes and
  AdtsHeader::write refuse what their fields cannot hold

Tests:
- the routing test checks one alignment for the ffmpeg version under test
- a default decode past an Ogg stream symphonia has no track for

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 8 (#443)

From wanikhawar's review:
- AudioSpecificConfig::parse reads backward-compatible SBR and PS signalling
  (a 0x2b7 sync extension after the config), which craft-media's Matroska
  demuxer writes for legacy HE-AAC IDs. Without lookahead those streams decoded
  as their core.
- An MP4 media edit after one empty edit is applied: its priming skip and its
  end. The empty edit's delay is not added as silence.
- A packet lost whole is concealed as long as the last one decoded, so a stream
  of multi-frame ADTS packets keeps its timing.
- ADTS-framed MP4 samples keep their sample table's timing, instead of every
  sample being read at open to look for several frames in one.
- AdtsHeader::write refuses an object type, sampling frequency index or channel
  configuration its fields cannot hold; it masked them into other values.
- AudioSpecificConfig::lc no longer panics for a sampling frequency index with no
  rate, and the decoder refuses such a config.
- AudioSpecificConfig::to_bytes refuses channel configuration 0 without a program
  config element, which would not parse back.
- Upsample2x starts each channel's history afresh when the channel count
  changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 9 (#443)

From CodeRabbit's review of round 8:
- AudioSpecificConfig::parse consumes extensionFlag3 after the config when
  extensionFlag is set, so the 0x2b7 sync extension after it is found.
- AudioSpecificConfig::to_bytes refuses a sampling rate of 0, which would not
  parse back.
- ADTS-framed HE-AAC in MP4 whose sample durations count core frames is not cut
  to the durations' half length: durations short of one frame per sample no
  longer end an ADTS-framed track, and its length is unknown until decoded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 10 (#443)

ADTS-framed AAC in MP4: durations short of the frames in the samples read
ahead no longer end the track. With HE-AAC samples of two ADTS frames each,
timed as two core frames, the durations matched one output frame per sample,
so the undercount check passed and the track stopped at half its length,
which probe reported as known. The frames each read-ahead sample holds are
now counted, and durations short of them leave the length unknown until
decoded, as durations short of one frame per sample already did. Correctly
timed AAC-LC samples of several frames keep their end trim.

Reported by FredApps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 11 (#443)

Corrects round 10. A short ADTS-framed MP4 track, whose last sample is in
the read-ahead, lost that sample's trim when it cut more than one frame:
the trim counted as durations falling short, so the length was left to
decoding and its padding was presented. The track's last sample is now left
out of the read-ahead comparison.

Reported by CodeRabbit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: document the ADTS-in-MP4 seek and edit limits (#443)

AAC in MP4 stored as ADTS frames, with sample durations that count fewer
frames than the samples hold, decodes in full from the start, but seeks
place samples by those durations and land later than asked, and an edit
list measured in them can end the track early. The README's Limits section
now says so, and the seek example points to it.

Reported by FredApps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: say why the ADTS-in-MP4 seek limit is kept (#443)

The Limits entry from 79f5839 said what goes wrong but not why it is left.
It now says that such a file is outside the MP4 specification: an MP4 AAC
sample is one raw access unit with the configuration in the sample
description (ISO/IEC 14496-14, 14496-3), so ADTS headers do not belong in
it, and a sample's duration is the time it decodes to (ISO/IEC 14496-12).
It says why files within the specification are not affected, what still
works, and why seeks and edits in such files are not corrected: finding
where each sample's audio starts means counting the ADTS frames in every
sample before it, one read per sample, which would slow every ADTS-framed
MP4 at open to work around files outside the specification.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: AAC encoder

FilmCraft's AAC-LC encoder behind `aac-encode`: psychoacoustic model,
window switching, M/S, TNS, bit reservoir, CBR and VBR, 1-8 channels,
optional ADTS framing. `rayon` encodes channels in parallel.
`aac::aac_order` maps WAV order to the encoder's input order. Settings
the encoder cannot use are `aac::Error::InvalidSettings`, which
converts to `AudioError::Encode`. `encode` now turns on `aac-encode`.

Tests that need AAC streams written by our own encoder come with it:
the encoder oracle and robustness tests, packet concealment, and
`TrackDecoder` on ADTS without extradata.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 1 (#444)

From CodeRabbit's first review of the AAC encoder:

- CBR bitrate limits use the exact bits per frame (bitrate * 1024
  against channels * sample_rate) instead of the truncated quotient, so
  288001 b/s at 48 kHz mono is rejected.
- Rates below 48 bits per channel per frame are rejected. That is the
  largest silent frame per channel measured across 1-8 channels; below
  it the encoder overshot the requested rate (96 kHz mono at 1000 b/s
  came out near 4500 b/s).
- Fill elements are capped so the byte-aligned access unit stays within
  6144 bits per channel. write_fill can write 8 bits over its request,
  which gave 769-byte mono units at the maximum rate and when the
  reservoir was flushed on the last frame.
- encode() buffers one frame of input at a time. Appending the whole
  slice and draining 1024 samples per frame copied the rest of the
  input once per frame, quadratic in the slice length. Output is
  byte-identical to feeding the input in chunks.
- README: 8 channels are taken in channel configuration 7's element
  order; a WAV 7.1 buffer has no equivalent there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 2 (#444)

From CodeRabbit's review of round 1:

- The CBR minimum adds the program config's bits when ADTS is on. The
  7-channel layout repeats it in every ADTS frame, and at full
  bandwidth a silent frame took 344 bits against the 336 bits that
  31,500 b/s gives at 96 kHz. Bandwidth alone stays within 48 bits
  per channel.
- write_fill never writes more than it is asked for: each element's
  size comes from the bits left, so the total ends 0-6 bits short.
  The old version could pass the request by 8 bits and could also
  stop early (asked for 14, it wrote 7).
- The encoder fills to the unit's target end, capped at
  max_frame_bits, instead of keeping round 1's 8-bit margin. Frames at
  the maximum rate were 767 bytes when there was little to code; they
  are now exactly 768 bytes per channel.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 3 (#444)

Encoder::new rejects a NaN VBR quality, the known issue ported from
FilmCraft and listed in the PR description.

NaN passes f32::clamp, so the rate-distortion multiplier became NaN.
The frame-size loop compares it with `<` and never ran, and no band
was skipped, so every frame was coded at full precision: mono units
reached 2465 bytes against the 768-byte limit, and 8-channel units of
up to 17024 bytes did not fit the ADTS length field, so they went out
without headers. Infinities and other out-of-range values are clamped
and stay within the limit; the test checks both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* craft-audio: review fixes, round 4 (#444)

From CodeRabbit's review of round 3:

- The CBR fill target is rounded down to a whole byte. Round 2 filled
  to an unaligned target, so after byte alignment a unit could end up
  to 7 bits past the bits it had: in a sweep of silent CBR encodes,
  1188 of 13200 frames overspent, all of them frames with fill, hidden
  by the level clamp at 0. The unit now ends exactly at the rounded
  target. The 0-7 bits rounded off stay in the reservoir and are left
  unspent after the last frame; the comment says so instead of
  claiming an exact total.
- The NaN-quality test checks that infinities and out-of-range values
  encode exactly as the nearest end of the 0.5-6.0 range.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-11 07:21:26 -05:00