Add audio capture for browser source

pkviet commented

2019-03-14 16:45:11 -05:00

(Migrated from github.com)

Relies on two cef commits (both on branch 3683):
58e1149 : Add ability to capture audio output to buffer via cef handler
1501768 : Mute audio in the browser (issue #1806)

This PR turns a browser source into a regular audio source, with a vue-meter on the Sound Mixer.

The audio from browser source is now muted to desktop.
It can be captured and monitored.
The PR is compatible with multiple audio sources in same browser page.

Main author: Osiris.
Other authors: Andersama & pkv

In memoriam. Osiris
Thanks for all that you did for this project. We deeply miss the
opportunity to keep working with you. - andersama , pkv & WizardCM

Co-authored-by: Michel Snippe michel@sokar.nl
Co-authored-by: Andersama anderson.john.alexander@gmail.com
Co-authored-by: pkv pkv.stream@gmail.com
Co-authored-by: WizardCM git@wizardcm.com

Relies on two cef commits (both on branch 3683): 58e1149 : Add ability to capture audio output to buffer via cef handler 1501768 : Mute audio in the browser (issue #1806) This PR turns a browser source into a regular audio source, with a vue-meter on the Sound Mixer. The audio from browser source is now muted to desktop. It can be captured and monitored. The PR is compatible with multiple audio sources in same browser page. Main author: Osiris. Other authors: Andersama & pkv **In memoriam. Osiris** Thanks for all that you did for this project. We deeply miss the opportunity to keep working with you. - andersama , pkv & WizardCM Co-authored-by: Michel Snippe <michel@sokar.nl> Co-authored-by: Andersama <anderson.john.alexander@gmail.com> Co-authored-by: pkv <pkv.stream@gmail.com> Co-authored-by: WizardCM <git@wizardcm.com>

❤️ 19

WizardCM commented

2019-03-14 17:47:27 -05:00

(Migrated from github.com)

Note: this requires heavy testing with CEF 3683 (Minimal Distribution) before merge, across all platforms.

Note: this requires heavy testing with [CEF 3683 (Minimal Distribution)](http://opensource.spotify.com/cefbuilds/index.html) before merge, across all platforms.

pkviet commented

2019-03-14 20:51:55 -05:00

(Migrated from github.com)

update

I've added checks for cef 3683.
Note that the first commit is required for cef >= 3683 and has a separate PR #158. Once the other commit is merged, I'll rebase to get rid of the duplicate commit.
added WizardCM as author, since he helped fix the compilation with cef 3683.
Note that cef 3683 breaks hardware acceleration

**update** - I've added checks for cef 3683. - Note that the first commit is required for cef >= 3683 and has a separate PR #158. Once the other commit is merged, I'll rebase to get rid of the duplicate commit. - added WizardCM as author, since he helped fix the compilation with cef 3683. - Note that cef 3683 breaks hardware acceleration

pkviet commented

2019-03-15 17:27:23 -05:00

(Migrated from github.com)

update

i've tested on cef master and cef 3683 where both audio commits are merged as well as hardware acceleration fixes.
Unfortunately, browser acceleration is still broken ...
branch 3626 has browser accel working but audio PR have not been merged to it.
the only build which has both audio and hardware acceleration is a custom patched build by Osiris.
We'll have to wait until this is fixed in cef.

**update** - i've tested on cef master and cef 3683 where both audio commits are merged as well as hardware acceleration fixes. Unfortunately, browser acceleration is still broken ... - branch 3626 has browser accel working but audio PR have not been merged to it. - the only build which has both audio and hardware acceleration is a custom patched build by Osiris. We'll have to wait until this is fixed in cef.

pkviet commented

2019-03-26 18:57:46 -05:00

(Migrated from github.com)

update

this PR for cef fixes hardware acceleration for cef 3683 and later (3729) : https://bitbucket.org/chromiumembedded/cef/pull-requests/222/fix-use_external_begin_frame-support-and/diff
I've compiled cef master (3729) as well as branch 3683 with this patch and obs-browser has both audio and hardware acceleration fully working.
However, a recent commit in master (chromium 3729) breaks api with CookieManager. Therefore panels are not working any more.
I have a separate fix for that here: https://gist.github.com/pkviet/3932d59e159ca4c8d03c89d228ddabcc
It's not PR'd because it requires adding some ifdefs to check cef version.
It would be ackward though to ifdef so many blocks of code and make for less readibility.
Maybe it'd be better to remove support for earlier versions of cef ...
That's something which will require some input from Jim.
on the 3683 branch the CookieManager fix is not needed.

**update** - this PR for cef fixes hardware acceleration for cef 3683 and later (3729) : https://bitbucket.org/chromiumembedded/cef/pull-requests/222/fix-use_external_begin_frame-support-and/diff - I've compiled cef master (3729) as well as branch 3683 with this patch and obs-browser has both audio and hardware acceleration fully working. - However, a recent commit in master (chromium 3729) breaks api with CookieManager. Therefore panels are not working any more. I have a separate fix for that here: https://gist.github.com/pkviet/3932d59e159ca4c8d03c89d228ddabcc It's not PR'd because it requires adding some ifdefs to check cef version. It would be ackward though to ifdef so many blocks of code and make for less readibility. Maybe it'd be better to remove support for earlier versions of cef ... That's something which will require some input from Jim. - on the 3683 branch the CookieManager fix is not needed.

RytoEX commented

2019-03-26 19:17:59 -05:00

(Migrated from github.com)

  It would be ackward though to ifdef so many blocks of code and make for less readibility.
  Maybe it'd be better to remove support for earlier versions of cef ...
  That's something which will require some input from Jim.

Personally, I'm totally for dropping official support for much older versions of CEF. I think the oldest version we should support (ifdefs, code paths, etc.) are the version we use for official obs-browser/obs-studio builds, or the oldest "Current Release Branch" of CEF, whichever is older. If the commits to remove older version support are clear and restricted to just dropping support for one version at a time, then it should be possible for others to revert those so that they can use older CEF builds if they want. As you noted, this is something Jim would have to decide. If we do decide to drop support for older CEF versions, I'd be happy to go through the codebase and remove sections specific to those dropped older versions.

> It would be ackward though to ifdef so many blocks of code and make for less readibility. > Maybe it'd be better to remove support for earlier versions of cef ... > That's something which will require some input from Jim. Personally, I'm totally for dropping official support for much older versions of CEF. I think the oldest version we should support (ifdefs, code paths, etc.) are the version we use for official obs-browser/obs-studio builds, or the oldest "Current Release Branch" of CEF, whichever is older. If the commits to remove older version support are clear and restricted to just dropping support for one version at a time, then it should be possible for others to revert those so that they can use older CEF builds if they want. As you noted, this is something Jim would have to decide. If we do decide to drop support for older CEF versions, I'd be happy to go through the codebase and remove sections specific to those dropped older versions.

👍 2

pkviet commented

2019-03-27 06:37:13 -05:00

(Migrated from github.com)

NB:
the CookieManager fix requires changes to obs UI too , cf : https://gist.github.com/pkviet/4a0fac632c835352d4f158d14641147e

NB: the CookieManager fix requires changes to obs UI too , cf : https://gist.github.com/pkviet/4a0fac632c835352d4f158d14641147e

SuslikV commented

2019-04-13 00:45:53 -05:00

(Migrated from github.com)

...
It would be ackward though to ifdef so many blocks of code and make for less readibility.
...

Such small files? Is in't better to if def them in include? And give a new name like "blah-blah-blah-3729.hpp" or similar?

> ... > It would be ackward though to ifdef so many blocks of code and make for less readibility. > ... Such small files? Is in't better to if def them in include? And give a new name like "blah-blah-blah-3729.hpp" or similar?

Show-vars commented

2019-05-24 01:40:45 -05:00

(Migrated from github.com)

Any good news? I apologize for the useless question, but I think many people are interested to know the current situation.

pkviet commented

2019-05-24 01:51:12 -05:00

(Migrated from github.com)

Any good news? I apologize for the useless question, but I think many people are interested to know the current situation.

yes and no.
The fix for hardware acceleration was merged into cef master BUT not to branches; there are issues with debug builds that the author has not addressed yet.
I tested release builds on branches with the fix and it seems to work (audio + hardware acceleration).
So we still need to wait. Another PR is in the works which rehauls the hardware acceleration.

> Any good news? I apologize for the useless question, but I think many people are interested to know the current situation. yes and no. The fix for hardware acceleration was merged into cef master BUT not to branches; there are issues with debug builds that the author has not addressed yet. I tested release builds on branches with the fix and it seems to work (audio + hardware acceleration). So we still need to wait. Another PR is in the works which rehauls the hardware acceleration.

❤️ 1

pkviet commented

2019-06-18 19:24:20 -05:00

(Migrated from github.com)

update June 19 2019
Some very good news.
The hardware acceleration fix has now been committed by Marshall in branch 3770.
(and branch 3729.)
But branch 3770 introduces a series of API changes (branch 3729 does too); I've added commits to fix them.
I checked that branch 3770 builds both with browser audio and hardware acceleration.
And that Twitch oauth is still working.

**update June 19 2019** Some very good news. The hardware acceleration fix has now been committed by Marshall in branch 3770. (and branch 3729.) But branch 3770 introduces a series of API changes (branch 3729 does too); I've added commits to fix them. I checked that branch 3770 builds both with browser audio and hardware acceleration. And that Twitch oauth is still working.

👍 4 👀 1 🎉 1

pkviet commented

2019-07-11 20:08:43 -05:00

(Migrated from github.com)

update July 12 2019

Applied clang-format to the PR.
One question though is that the PR is littered by CHROME_VERSION_BUILD checks.
This makes the code unnecessarily convoluted.
It would be simpler as discussed above to decide to support only the last cef release.
the PR was checked to be compatible with proprietary codecs builds of cef (supporintg h264/aac)

**update July 12 2019** - Applied clang-format to the PR. - One question though is that the PR is littered by CHROME_VERSION_BUILD checks. This makes the code unnecessarily convoluted. It would be simpler as discussed above to decide to support only the last cef release. - the PR was checked to be compatible with proprietary codecs builds of cef (supporintg h264/aac)

WizardCM commented

2019-07-11 20:11:24 -05:00

(Migrated from github.com)

I agree that starting with these changes, we should support 3770 at the minimum going forward. There is no reason to support older builds. This would require testing macOS with the latest CEF too.

pkviet commented

2019-07-13 11:22:18 -05:00

(Migrated from github.com)

update July 13

removed the commits related to cef 3770 fixes (Jim implemented them separately)
removed the cef 3683 fix (merged by Jim)
rebased against current master, works perfectly with cef 3770

**update July 13** - removed the commits related to cef 3770 fixes (Jim implemented them separately) - removed the cef 3683 fix (merged by Jim) - rebased against current master, works perfectly with cef 3770

dragonbane0 commented

2019-07-13 17:07:20 -05:00

(Migrated from github.com)

I pray that this gets merged soon - we need it badly lol

👍 1

pkviet commented

2019-07-18 07:31:52 -05:00

(Migrated from github.com)

update

fixed a timestamp issue (was in microseconds instead of nanoseconds due to an API change when the cef PR was merged in master). Thanks to Jim for solving the bug.

**update** - fixed a timestamp issue (was in microseconds instead of nanoseconds due to an API change when the cef PR was merged in master). Thanks to Jim for solving the bug.

Rows
Columns

Add audio capture for browser source #157