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

Emscripten audio video revision #7217

Merged

Conversation

Jonathhhan
Copy link
Contributor

This is basically what I changed end of last year.
I compared it with my recent changes and it seems more complete (but I changed a lot).
I can also split it up into smaller parts.

@Jonathhhan Jonathhhan mentioned this pull request Nov 30, 2022
@Jonathhhan Jonathhhan closed this Dec 1, 2022
@Jonathhhan
Copy link
Contributor Author

Its not really a pull request, its more like a suggestion. It can be surely further optimized, but works quite well.

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Dec 2, 2022

Let my try to explain what the main changes are:

Audio:
There is only one audioContext().
There is only one fft.
There is only one stream.
There is only one media element.
Added soundPans: [].
There are no soundBuffers: [], no soundSources: [], no soundStartTimes: [], and no soundGains: [].

Video:
Added soundSources: [], added soundPans: []. This has the advantage that its possible to pass the audio from the video into OF for further processing, which is also possible with the stream or the soundplayer (at least with audioworklets ;) ).

Please tell me, if I reduced too much, but until now everything seems to work as before (plus a few additional features like pan).

@Jonathhhan Jonathhhan reopened this Dec 2, 2022
@Jonathhhan Jonathhhan closed this Dec 2, 2022
@Jonathhhan Jonathhhan force-pushed the Emscripten-audioVideo-Revision branch from 3874d87 to 33370bf Compare December 2, 2022 19:42
@Jonathhhan Jonathhhan reopened this Dec 2, 2022
@ofTheo
Copy link
Member

ofTheo commented Dec 13, 2022

@Jonathhhan thanks for this!
From a user perspective is everything the same? Are there things that used to work that won't work with this update?
Or does this PR just add new features or provide more options?

@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Dec 13, 2022

@ofTheo the idea is, to remove redundant stuff, fix some stuff and add some features.
From a user perspective everything is the same (+ some fixes and features), at least that is what I experienced from testing.
One of the main changes is, that I removed the soundBuffers, so the soundfile is accessed directly from the blob url (an advantage from that is, that it is possible to load very large soundfiles without increasing the memory too much).

@ofTheo
Copy link
Member

ofTheo commented Dec 13, 2022

Okay great @Jonathhhan 👍 - if you want to just let me know when this is good to merge.
I think currently there is a conflict in ofxEmscriptenVideoGrabber.cpp

@ofTheo ofTheo merged commit 281b1b1 into openframeworks:master Dec 15, 2022
@Jonathhhan
Copy link
Contributor Author

Jonathhhan commented Dec 15, 2022

@ofTheo thank you for merging this. Actually I wanted to triple check it, to be sure there are no errors. But I checked it once again yesterday, and did not found anything so far. Maybe its better like that, instead of waiting for too long. The conflict with the grabber was just a space.

@ofTheo
Copy link
Member

ofTheo commented Dec 15, 2022

Ahh yes! Was wondering if it was done.
It could always be patched if anything comes up and probably good to get it in for testing too.
🙂

@Jonathhhan Jonathhhan deleted the Emscripten-audioVideo-Revision branch March 4, 2023 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants