Skip to content

Conversation

@NicolasHug
Copy link
Contributor

@NicolasHug NicolasHug commented Apr 9, 2025

This PR adds support for encoding formats where the encoder doesn't natively support FLTP, which is the sample format of the input waveform. We use swresample to convert the input FLTP AVFrames into a sample format that the encoder support.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Apr 9, 2025
@NicolasHug NicolasHug marked this pull request as ready for review April 9, 2025 12:40
avCodecContext_->sample_rate = sampleRate;

// Note: This is the format of the **input** waveform. This doesn't determine
// the output.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My original comment was wrong: that's not the format of the input waveform. It's the format of the input AVFrame that we pass to avcodec_send_frame(). And it needs to be a format that the codec supports.

avCodecContext_->frame_size > 0,
"frame_size is ",
avCodecContext_->frame_size,
". Cannot encode. This should probably never happen?");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This won't always be non-zero, see below.

Copy link
Contributor

@scotts scotts Apr 11, 2025

Choose a reason for hiding this comment

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

Interesting - it might be worth noting why from the docs (https://ffmpeg.org/doxygen/6.0/structAVCodecContext.html#aec57f0d859a6df8b479cd93ca3a44a33, which I admit to not understanding) when we turn 0 into our default.

# Check that decode(encode(samples)) == samples on lossless formats

if get_ffmpeg_major_version() == 4 and output_format == "wav":
pytest.skip("Swresample with FFmpeg 4 doesn't work on wav files")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@NicolasHug NicolasHug merged commit 2c137e7 into meta-pytorch:main Apr 14, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants