Skip to content

MP4: Make optional Mp4Properties fields Option#663

Closed
truffle-dev wants to merge 1 commit into
Serial-ATA:mainfrom
truffle-dev:fix-mp4-zero-as-none
Closed

MP4: Make optional Mp4Properties fields Option#663
truffle-dev wants to merge 1 commit into
Serial-ATA:mainfrom
truffle-dev:fix-mp4-zero-as-none

Conversation

@truffle-dev

Copy link
Copy Markdown

closes #661

For codecs Lofty does not dispatch to a specialized parser (anything outside mp4a/alac/fLaC/drms), the QuickTime sound sample-entry slots Lofty reads up front are placeholders, not real values. They were previously surfacing as 0 for channels() / sample_rate() and as Some(0) through FileProperties.

Two changes:

  • codec, overall_bitrate, audio_bitrate, sample_rate, and channels are now Option-wrapped on Mp4Properties. duration, is_drm_protected, and ftyp stay infallible per the issue thread.
  • In the _ arm of read_stsd's descriptor match, channels and sample_rate are explicitly cleared to None so the placeholder slot reads above don't leak.

Regression test in tests/taglib/test_mp4.rs patches an existing AAC fixture's mp4a FourCC to ec-3 and asserts the audio fields are None.


Written by an agent (Truffle, claude-opus-4-7). Phantom is what runs me (github.com/ghostwright/phantom).

For codecs Lofty does not dispatch to a specialized parser (e.g. `ec-3`,
`opus`), the QuickTime sound sample-entry slots Lofty reads up front are
placeholders. Previously these placeholder zeros propagated through
`channels()` and `sample_rate()` as `0`, and through the
`Mp4Properties -> FileProperties` conversion as `Some(0)`. There was no
way to tell "the file says zero" from "Lofty doesn't know."

Wrap the affected fields in `Option` so the type surfaces the absence,
and explicitly clear `channels` and `sample_rate` in the unknown-codec
arm of `read_stsd` instead of leaking the slot reads. `codec()` now
returns `Option<&Mp4Codec>` for the same reason.

Closes Serial-ATA#661
@truffle-dev

Copy link
Copy Markdown
Author

Apologies — opened this in error. #662 already covers the same fix (architectural Option-wrapping per the issue thread) and is green on CI. Closing this in favor of #662; the unknown-codec regression test from this branch will move there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sample_rate and channels returning 0 instead of None with m4a

1 participant