Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding extended precision ByteVector conversion for AIFF sample rate #245

Merged
merged 1 commit into from
Jan 24, 2022
Merged

Adding extended precision ByteVector conversion for AIFF sample rate #245

merged 1 commit into from
Jan 24, 2022

Conversation

benrr101
Copy link
Contributor

Description: During my port of taglib# to node, I noticed a long string of logic for determining the sample rate for AIFF files. Since it's not uncommon for sample rates to be stored in a table, I didn't think much of it. However, while working on writing unit tests for the Aiff.StremHeader class, I wanted to double check the AIFF spec to see if there were any more sample rates, since it seemed odd that AIFF could only support a max sample rate of 64kHz when 96kHz and 192kHz files are all the rage these days. Turns out the AIFF spec uses an 80-bit IEEE 754 extended precision floating point number for storing the sample rate (http://muratnkonar.com/aiff/index.html), not a lookup table of sample rates. What's more, I've seen Apple users mention that 96kHz AIFF files exist (https://discussions.apple.com/thread/7273841). Therefore, the existing logic is going to give faulty duration and sample rate values for AIFF files with sample rates > 64kHz.

The proposed solution in this PR is to add logic for converting a ByteVector into a IEEE 754 extended precision floating point value, expressed as a double. The Aiff.StreamHeader class has been updated to use this to get the full sample rate. The basis for this logic was from http://www33146ue.sakura.ne.jp/staff/iz/formats/ieee.c

Since this is the only usage of this new method, I'm open to storing it somewhere other than the ByteVector class.

Testing:

  • Existing tests for AIFF continue to work.
  • I feel like there should be thorough tests of the new ByteVector method, but I'm not entirely sure how to fit it into the existing set of tests for the class. Any suggestions would be welcome.

Using extended precision conversion to get any possible AIFF sample rate
Base automatically changed from master to main March 10, 2021 16:12
@decriptor decriptor merged commit 22c635c into mono:main Jan 24, 2022
@benrr101 benrr101 deleted the feature/full-aiff-samplerate branch November 1, 2023 02:21
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.

2 participants