Skip to content

refactor: decoder utilizing ffmpeg and ma decoders#1050

Open
mdydek wants to merge 3 commits intomainfrom
refactor/decoder
Open

refactor: decoder utilizing ffmpeg and ma decoders#1050
mdydek wants to merge 3 commits intomainfrom
refactor/decoder

Conversation

@mdydek
Copy link
Copy Markdown
Collaborator

@mdydek mdydek commented May 6, 2026

Closes #

⚠️ Breaking changes ⚠️

Introduced changes

  • decoder now utilizes already present ffmpeg and miniaudio decoding code

Checklist

  • Linked relevant issue
  • Updated relevant documentation
  • Added/Conducted relevant tests
  • Performed self-review of the code
  • Updated Web Audio API coverage
  • Added support for web
  • Updated old arch android spec file

Copy link
Copy Markdown
Contributor

@closetcaiman closetcaiman left a comment

Choose a reason for hiding this comment

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

lgtm,

nice job deduplicating this!

Comment thread packages/react-native-audio-api/common/cpp/audioapi/core/utils/AudioDecoder.cpp Outdated
@mdydek mdydek requested a review from closetcaiman May 8, 2026 10:14
Copy link
Copy Markdown
Contributor

@closetcaiman closetcaiman left a comment

Choose a reason for hiding this comment

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

Nice job including the decoder-specific errors. I also like the namespace usage instead of a class with static methods, it feels a lot more cpp-like and modern. I left a few nitpicks.

const ma_uint32 outRate =
outputSampleRate > 0 ? static_cast<ma_uint32>(outputSampleRate) : 0;
ma_decoder_config config = ma_decoder_config_init(ma_format_f32, 0, outRate);
static ma_decoding_backend_vtable *customBackends[] = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

from clang-tidy: Do not declare C-style arrays, use 'std::array' instead

either resolve or ignore if it's ma-specific, but add a comment.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

miniaudio is basically c library, so I think in this case we would like to leave it as it is

ma_decoding_backend_libvorbis,
ma_decoding_backend_libopus,
};
config.ppCustomBackendVTables = customBackends;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

from clang-tidy: Do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead

either resolve or ignore if it's ma-specific, but add a comment.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

miniaudio is basically c library, so I think in this case we would like to leave it as it is

Comment on lines +24 to +32
AudioFormat detectAudioFormat(const void *data, size_t size);

static constexpr int CHUNK_SIZE = 4096;
bool pathHasExtension(const std::string &path, const std::vector<std::string> &extensions);

class AudioDecoder {
public:
AudioDecoder() = delete;
inline bool needsFFmpeg(AudioFormat format) {
return format == AudioFormat::MP4 || format == AudioFormat::M4A || format == AudioFormat::AAC;
}

[[nodiscard]] static AudioBufferResult decodeWithFilePath(
const std::string &path,
float sampleRate);
[[nodiscard]] static AudioBufferResult
decodeWithMemoryBlock(const void *data, size_t size, float sampleRate);
[[nodiscard]] static AudioBufferResult decodeWithPCMInBase64(
const std::string &data,
float inputSampleRate,
int inputChannelCount,
bool interleaved);
inline bool needsFFmpegByPath(const std::string &path) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aren't those [[nodiscard]] too?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants