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 support for RF64 wave files #9543

Merged
merged 4 commits into from
Nov 11, 2021
Merged

Add support for RF64 wave files #9543

merged 4 commits into from
Nov 11, 2021

Conversation

KasemJaffer
Copy link
Contributor

This commit adds support for RF64 wave files without breaking the existing support for RIFF wave files

Copy link
Contributor

@kim-vde kim-vde left a comment

Choose a reason for hiding this comment

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

It would be great if you could add a unit test for RF64 in WavExtractorTest! Is it possible to generate a RF64 file that is fairly small in size (ideally less than 100KB)?

@KasemJaffer
Copy link
Contributor Author

I added a unit test and fixed a bug i found along the way.

@@ -53,4 +53,10 @@ public void sample_imaAdpcm() throws Exception {
ExtractorAsserts.assertBehavior(
WavExtractor::new, "media/wav/sample_ima_adpcm.wav", simulationConfig);
}

@Test
public void sample_RF64() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

You also need to generate the dump files that go with the test. To do so, you need to,

  • Set DUMP_FILE_ACTION to WRITE_TO_LOCAL in DumpFileAsserts.java
  • Run the test. This will create dump files that are a summary of the data extracted by the WavExtractor.
  • Set DUMP_FILE_ACTION back to COMPARE_WITH_EXISTING.
  • Run the test. This will compare the extracted data with the files previously generated. This test need to pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added the dump files and the test is passing :)

tonihei pushed a commit that referenced this pull request Nov 1, 2021
This refactoring is the basis to support RF64 (see
Issue: #9543).

#minor-release

PiperOrigin-RevId: 406377924
@KasemJaffer
Copy link
Contributor Author

I don't know why its saying there is a conflicting file. There shouldn't be because I have rebased my branch on the latest dev-v2 commit.

@kim-vde
Copy link
Contributor

kim-vde commented Nov 2, 2021

Hey! I am currently working on merging this PR. I had to make some refactorings to make the code 100% correct. You don't need to do anything. The code PR should be merged in the following days/weeks.

tonihei pushed a commit that referenced this pull request Nov 3, 2021
This refactoring is the basis to support RF64 (see
Issue: #9543).

#minor-release

PiperOrigin-RevId: 407301056
tonihei pushed a commit that referenced this pull request Nov 4, 2021
This refactoring is the basis to support RF64 (see
Issue: #9543).

#minor-release

PiperOrigin-RevId: 407301056
icbaker pushed a commit to androidx/media that referenced this pull request Nov 9, 2021
This refactoring is the basis to support RF64 (see
Issue: google/ExoPlayer#9543).

#minor-release

PiperOrigin-RevId: 406377924
icbaker pushed a commit to androidx/media that referenced this pull request Nov 9, 2021
This refactoring is the basis to support RF64 (see
Issue: google/ExoPlayer#9543).

#minor-release

PiperOrigin-RevId: 407301056
@tonihei tonihei merged commit 04517e1 into google:dev-v2 Nov 11, 2021
tonihei added a commit that referenced this pull request Nov 11, 2021
PiperOrigin-RevId: 408816643
@google google locked and limited conversation to collaborators Jan 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants