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

Why not use audio context? #14

Open
adi518 opened this issue Sep 23, 2021 · 4 comments
Open

Why not use audio context? #14

adi518 opened this issue Sep 23, 2021 · 4 comments

Comments

@adi518
Copy link

adi518 commented Sep 23, 2021

The following method is leaner and absolves creating a DOM element.

const context = new AudioContext();
const buffer = await file.arrayBuffer();
const audio = await context.decodeAudioData(buffer);

context.close()

console.log(audio.duration); // output: 4.86 (seconds)

Also, your library is not fully externalized, it's bundling Babel helpers, which increases its size from 700b to 2.8kb. See https://bundlephobia.com/package/[email protected].

@evictor
Copy link
Owner

evictor commented Dec 22, 2022

Thanks for your feedback. Apologies for the delayed reply.

How does the footprint and exec. time of creating a DOM element compare to those of AudioContext, etc.? How have you come to the conclusion that the latter is leaner?

Another thing to consider is that a single DOM element being created is of negligible weight under normal circumstances, i.e. how I anticipate this library to be used, that is, not in tight loops or repeatedly called thousands of times.

Finally, note the element is not attached to the DOM, which is probably where any "real" footprint becomes apparent (again, only at n ≫ 1).

Are you able to submit a patch for the bundle issue?

Thanks again.

@adi518
Copy link
Author

adi518 commented Dec 22, 2022

It can probably be benchmarked, but I'd say it makes more sense to use a natively available API than work around it with the DOM API. As far as the package size goes, I don't have time for it atm, but you can try substituting Gulp with Rollup, that should take care of most of it.

@evictor
Copy link
Owner

evictor commented Dec 23, 2022

Thanks, will look into that. 👍🏻 Re: reason for DOM API, there is none other than it worked to get around the browser limitation, and alternatives were not apparent at the time. This workaround applies to a specific limitation of Chrome regarding reading the duration of just-recorded, in-memory webm videos, for which duration is not available in the webm video's metadata. The video meta is standards compliant so Google was not interested in "fixing" the video writer at an appropriate lower level, so I figured out a way to force the issue with the DOM involvement.

It might be the case—I might even assume this is the case—that AudioContext, too, suffers from the same limitation and therefore may not be a viable workaround for the core issue. In fact I would be quite surprised if at the time I didn't already attempt usage of AudioContext; as a guiding principle I try to exhaust all options in order of increasing complexity/esotericity when considering workarounds to problematic code that can't be changed.

So with that in mind, were you able to reproduce the original limitation, then show that AudioContext can solve it?

@jonatanvm
Copy link

The following method is leaner and absolves creating a DOM element.

const context = new AudioContext();
const buffer = await file.arrayBuffer();
const audio = await context.decodeAudioData(buffer);

context.close()

console.log(audio.duration); // output: 4.86 (seconds)

Also, your library is not fully externalized, it's bundling Babel helpers, which increases its size from 700b to 2.8kb. See https://bundlephobia.com/package/[email protected].

The original solution does not work in Firefox anymore, but @adi518 proposal does 👍

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

No branches or pull requests

3 participants