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

Add ADPCM Decoder #160

Merged
merged 9 commits into from
Nov 8, 2022
Merged

Add ADPCM Decoder #160

merged 9 commits into from
Nov 8, 2022

Conversation

geckoxx
Copy link
Contributor

@geckoxx geckoxx commented Nov 4, 2022

This adds an ADPCM decoder with following limitations:

  • only Microsoft ADPCM and IMA ADPCM codecs
  • only mono and stereo channels
  • only 4bits per sample (IMA ADPCM allows 3bits)

It also expands the Wave Format to support ADPCM and the packetization over a number of samples (PCM) or blocks (ADPCM).
To support multiple data blocks per packet CodecParameters has a new parameter: frames_per_block.

Related Issue: #124
Resolves: #41

Copy link
Owner

@pdeljanov pdeljanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, this is really great! Thanks for implementing these decoders!

There are some minor changes that need to be made, and a couple other things that I think need to be double checked, but overall it looks good to me.

A couple general items:

  1. Please add yourself to the CONTRIBUTORS file
  2. It's not very obvious, but you need to run a nightly version of cargo fmt for the code to be formatted correctly. The CI will complain otherwise.

In terms of testing, have you ran your test files through symphonia-check?

I transcoded a couple files to ADPCM and checked them, but that's a very small test.

Thanks again, and hope to get this merged soon!

symphonia-codec-adpcm/README.md Outdated Show resolved Hide resolved
symphonia-codec-adpcm/README.md Outdated Show resolved Hide resolved
symphonia-codec-adpcm/README.md Show resolved Hide resolved
symphonia-codec-adpcm/src/codec_ima.rs Show resolved Hide resolved
symphonia-codec-adpcm/src/codec_ms.rs Show resolved Hide resolved
symphonia-codec-adpcm/src/codec_ms.rs Outdated Show resolved Hide resolved
symphonia-codec-adpcm/src/codec_ms.rs Outdated Show resolved Hide resolved
symphonia-codec-adpcm/src/common.rs Outdated Show resolved Hide resolved
symphonia-codec-adpcm/src/lib.rs Outdated Show resolved Hide resolved
symphonia-format-wav/src/chunks.rs Show resolved Hide resolved
@torokati44
Copy link

Just FYI, FWIW there's also an ADPCM implementation here: https://github.com/ruffle-rs/ruffle/blob/master/core/src/backend/audio/decoders/adpcm.rs

It might be too simplistic (it has to support only what Flash Player did), but it could still be interesting for cross-referencing.

@geckoxx
Copy link
Contributor Author

geckoxx commented Nov 7, 2022

Just FYI, FWIW there's also an ADPCM implementation here: https://github.com/ruffle-rs/ruffle/blob/master/core/src/backend/audio/decoders/adpcm.rs

It might be too simplistic (it has to support only what Flash Player did), but it could still be interesting for cross-referencing.

I will look into it. Seems to be the IMA Codec.

@geckoxx
Copy link
Contributor Author

geckoxx commented Nov 7, 2022

I tested my 2868 files (all with the Microsoft ADPCM Codec) and all passed.
I tested also the 16 ADPCM files from this side and all passed.

@pdeljanov
Copy link
Owner

@geckoxx, thanks for making those changes.

Things look good to me, and your test results are very promising. Once you are ready we can merge the decoder and let it sit in the main branch for a bit.

I'm sure some people will experiment with it, and perform further testing.

@geckoxx geckoxx marked this pull request as ready for review November 8, 2022 07:45
@pdeljanov pdeljanov merged commit 4c508b2 into pdeljanov:master Nov 8, 2022
@pdeljanov
Copy link
Owner

Thanks for your work, @geckoxx! Merged!

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.

ADPCM support
3 participants