kbumsik / opus-media-recorder

MediaRecorder polyfill for Opus recording using WebAssembly
http://kbumsik.io/opus-media-recorder/
Other
318 stars 39 forks source link

Multiple use of MediaRecorder creates too many AudioContext objects #18

Closed janinou closed 5 years ago

janinou commented 5 years ago

Hi Kim, First of all thank you, as we're able to solve the problem of uploading audio to our servers from iPad browsers, which is key in our applications :-)

I however had an issue of inconsistent behaviour. After making several calls to the MediaRecorder class (due to recording multiple times in a row), I got an error popping.

TypeError: null is not an object (evaluating 't.context.sampleRate')

The reason for that is the fact we're creating a new AudioContext at each MediaRecorder call: this.context = new AudioContext();It seems there is a browser limitation to how many AudioContext can be built (see here and there).

Edit This patch allows to remove multiple audio context creation, but keeping the creation of the initial AudioContext within the fallback of a user-gesture.

kbumsik commented 5 years ago

Hi, I'm happy to hear you find it helpful :)

Thank you very much for reporting. However, I can not accept your patch because this patch will disallow users to create multiple MediaRecorders at a time, and I'm not sure if running multiple MediaRecorder is important.

Another possible fix is that closing the AudioContext when you call MediaRecorder.close(). Do you think it will resolve the issue too? I will test this fix this weekend.

BTW could you tell me roughly how many new MediaRecorder objects will start causing the problem? I will add this as a test case.

janinou commented 5 years ago

I'm not sure this would disallow to create multiple at a time (at least I was able to create 2). But anyway I'm not very proud of the patch :-)

My use case is:

So I guess closing the MediaRecorder and the AudioContext doing so would probably work :-)

prtcl commented 5 years ago

hi! I've run into this same issue. the above patch might resolve it, or also maybe consider adding a non-standard constructor option to pass in your own context instance. in my app there is already a context running, so it's easy to reuse the same one. I am not sure how many others have this same use case though

kbumsik commented 5 years ago

@BenjaminAbdi @prtcl I believe ee2fa8a fixes the problem. The new release 0.7.18 have the patch. Could you test if the patch worked for you? It took some days to apply this patch because I cannot reproduce the issue (though it looks obvious). So I'm not 100% sure about this patch. It would be great if I can get any minimal example that can reproduce the problem.

@prtcl I'm not sure about the suggestion. The goal of this project it to reimplement MediaRecorder therefore I would be very cautious about adding a non-standard option unless inevitable. Let's see the patch works first.

janinou commented 5 years ago

@kbumsik Your patch solves the problem, thanks a lot :-)