Skip to content

feat: preloading in audio tag#1075

Open
mdydek wants to merge 4 commits into
mainfrom
feat/audio-tag-preload
Open

feat: preloading in audio tag#1075
mdydek wants to merge 4 commits into
mainfrom
feat/audio-tag-preload

Conversation

@mdydek
Copy link
Copy Markdown
Collaborator

@mdydek mdydek commented May 19, 2026

Closes #

⚠️ Breaking changes ⚠️

Introduced changes

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

@mdydek mdydek added the feature New feature label May 19, 2026
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!

I'm not a fan of the current state of Audio.tsx. Maybe we can consider refactoring it for readability in the future?

Comment thread packages/react-native-audio-api/src/development/react/Audio/Audio.tsx Outdated
Comment thread packages/react-native-audio-api/src/development/react/Audio/Audio.tsx Outdated
Comment thread packages/react-native-audio-api/src/utils/metadataPrefetching.ts
Comment thread packages/react-native-audio-api/src/development/react/Audio/utils.ts Outdated
Comment thread packages/react-native-audio-api/common/cpp/audioapi/libs/ffmpeg/FFmpegDecoding.h Outdated
@mdydek mdydek requested a review from closetcaiman May 20, 2026 13:49
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.

Small nitpicks. JD overall!

Comment on lines +200 to +203
template <typename T>
concept Decoder = std::is_base_of_v<decoding::IncrementalAudioDecoder, T>;

template <Decoder D>
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.

Suggested change
template <typename T>
concept Decoder = std::is_base_of_v<decoding::IncrementalAudioDecoder, T>;
template <Decoder D>
template <typename D>
requires std::is_base_of_v<decoding::IncrementalAudioDecoder, D>

Comment on lines +23 to +25
auto duration = std::optional<double>();
duration =
audiodecoding::probeDuration<miniaudio_decoder::MiniAudioDecoder>(data, size, sampleRate);
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.

Suggested change
auto duration = std::optional<double>();
duration =
audiodecoding::probeDuration<miniaudio_decoder::MiniAudioDecoder>(data, size, sampleRate);
auto duration =
audiodecoding::probeDuration<miniaudio_decoder::MiniAudioDecoder>(data, size, sampleRate);

): Promise<number | null> {
return this.fileUtils.probeDuration(data, sampleRate);
}
}
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.

Is this used anywhere else? I think it can be safely removed and the fetching line can be moved to probeDuration. Also, remember to use await here.

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.

Ughh, the line selection was cut. The full function is:

public async probeDurationInstance(
    data: ArrayBuffer,
    sampleRate?: number
  ): Promise<number | null> {
    return await this.fileUtils.probeDuration(data, sampleRate);
  }

Comment on lines +20 to 29
const normalizedPreload = props.preload || 'auto';

return {
...props,
autoPlay: props.autoPlay ?? false,
controls: props.controls ?? false,
loop: props.loop ?? false,
muted: props.muted ?? false,
preload: props.preload ?? 'auto',
preload: normalizedPreload,
source: props.source ?? [],
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.

Suggested change
const normalizedPreload = props.preload || 'auto';
return {
...props,
autoPlay: props.autoPlay ?? false,
controls: props.controls ?? false,
loop: props.loop ?? false,
muted: props.muted ?? false,
preload: props.preload ?? 'auto',
preload: normalizedPreload,
source: props.source ?? [],
return {
...props,
autoPlay: props.autoPlay ?? false,
controls: props.controls ?? false,
loop: props.loop ?? false,
muted: props.muted ?? false,
preload: props.preload || 'auto',
source: props.source ?? [],

Comment on lines +54 to +55
const preloadMode: PreloadType =
preload === 'none' || preload === 'metadata' ? preload : 'auto';
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.

Is this even necessary if we use useStableAudioProps?

Comment on lines +583 to +585
std::optional<double> FFmpegDecoder::probeDuration(const void *, size_t, int) {
return std::nullopt;
}
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.

This is not defined in the header. What is this for?

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

Labels

feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants