screenshare: add rap processing for screen streams with audio (complements #11622) #11626

prlanzarin · 2021-03-11T23:48:11Z

What does this PR do?

Lifts @germanocaumo's (Mconf) work on making RaP correctly process raw screen sharing webm files with audio tracks in them.
Complements #11622 by adding the full recording capability to it.

Closes Issue(s)

#8632 (combined with #11622)

More

A few things about this:

Uses libopus to handle the decoding of the webm audio tracks. I'm almost certain it is built with our 2.3 ffmpeg package?
There's a resample in the audio track processing to correct any stream gaps that the raw might have and thus alleviate A/V desync on lossy streams
The code only kicks in if the webm has an audio track in it, otherwise business as usual
The audio track is mixed with the full audio recording file at the end

use libopus decoder and encoder, its better than built-in ffmpeg/flac don't mix screenshare audio with mics, was generating desync with bad audio segments, encode it together with video file (TODO: needs adjustments in playback)

…lems in iOS, fixed desync with aresample

kepstin · 2021-03-12T20:24:29Z

Hmm. I don't really want to do the audio mixing with an extra step after rendering like this - instead, the render function should be modified to allow multiple parallel audio streams in the edl that it mixes together during the render as a single step. This will speed it up, and also remove a generation of lossy encoding. This would be analogous to how the video rendering code supports multiple "areas".

There's no reason for there to be a separate internal audio format for screensharing audio. We should probably just switch everything to opus.

There's some complications about the volume levels when using amix like this - by default it attenuates both audio sources when mixing, so the result will have noticeably lower volume in the sections when only one of the two audio sources was active. (The ability to disable that function is only present in unreleased git ffmpeg)

kepstin · 2021-03-12T20:28:37Z

record-and-playback/core/lib/recordandplayback/edl/audio.rb

@@ -155,6 +185,7 @@ def self.render(edl, output_basename)
          if audioinfo[input[:filename]][:format][:format_name] == 'wav'
            ffmpeg_cmd += ['-ignore_length', '1']
          end
+          screenshare ? ffmpeg_cmd += ['-c:a', 'libopus'] : nil


Why are you using the libopus decoder here instead of ffmpeg's builtin opus decoder? Have you had any specific issues with the builtin decoder causing problems?

Yes. Higher frequency of A/V de-sync occurrences and a bit worse handling of lossy raw streams.
Don't have any data on that anymore, though.

Can you confirm whether libopus is enabled in the ffmpeg package 2.3 is using?

If you're having problems like that with the internal opus decoder, we should probably switch decoding all opus to use libopus. In any case, selecting the libopus decoder needs to be made conditional on the audioinfo saying that the codec is 'opus'.

I added the libopus decoder to the ffmpeg build a while back, search for "Enabled decoders" in the build log to see what's there: https://launchpadlibrarian.net/500061740/buildlog_ubuntu-bionic-amd64.ffmpeg_7%3A4.2.4-1ubuntu0.1bbb2~18.04_BUILDING.txt.gz

kepstin · 2021-03-12T20:41:08Z

record-and-playback/core/lib/recordandplayback/edl/audio.rb

@@ -22,8 +22,11 @@ module EDL
    module Audio
      FFMPEG_AEVALSRC = "aevalsrc=s=48000:c=stereo:exprs=0|0"
      FFMPEG_AFORMAT = "aformat=sample_fmts=s16:sample_rates=48000:channel_layouts=stereo"
+      FFMPEG_AFORMAT_SCREENSHARE = "aresample=async=1000,aformat=sample_fmts=s16:sample_rates=48000:channel_layouts=stereo"


Thinking about it a bit, we should probably apply the aresample=async=XXX filter to all of the audio inputs. It won't hurt anything and who knows, it might help improve audio sync in a few cases?

it might help improve audio sync in a few cases

Yeah. While I didn't implement this, I suggested the resample to try and tackle desync issues @germanocaumo was dealing with on streams riddled with gaps due to simulated packet drops. It did improve it by a large margin. So might be a good idea to put it in the other inputs as well yes.

I'd do it in a separate PR if possible?

I'd like this PR to use the same audio format for all audio, there's no reason to special-case the screenshare audio.

This incorporates only the audio desync related changes from bigbluebutton#11626 * Add the aresample filter with async option to fill in timestamp gaps * Use the libopus decoder for opus audio instead of ffmpeg's builtin decoder

github-actions · 2021-03-18T15:49:05Z

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

into u23-recsa

kepstin · 2021-03-24T14:12:24Z

record-and-playback/core/lib/recordandplayback/edl/audio.rb

+        inputs.each do |input|
+          ffmpeg_cmd += ['-i', input]
+        end
+        ffmpeg_cmd += ['-filter_complex', "amix"]


We should explicitly set the number of inputs to the amix filter here, in case a number of inputs ≠ 2 is provided.

Suggested change

ffmpeg_cmd += ['-filter_complex', "amix"]

ffmpeg_cmd += ['-filter_complex', "amix=inputs=#{inputs.length}"]

kepstin · 2021-03-24T14:15:15Z

record-and-playback/core/lib/recordandplayback/generators/audio.rb

+        :audio => nil
+      }
+
+      events.xpath('/recording/event[@module="Deskshare" or (@module="bbb-webrtc-sfu" and (@eventname="StartWebRTCDesktopShareEvent" or @eventname="StopWebRTCDesktopShareEvent"))]').each do |event|


I don't think the "Deskshare" module (I think that's the old java screensharing?) can ever have audio, so this should only look at the WebRTC events.

germanocaumo added 5 commits March 11, 2021 21:09

Re-add screenshare audio recording processing

0d48dd0

Improve screenshare audio recording sync:

fdbbffb

use libopus decoder and encoder, its better than built-in ffmpeg/flac don't mix screenshare audio with mics, was generating desync with bad audio segments, encode it together with video file (TODO: needs adjustments in playback)

Mix screenshare audio with mics again to avoid playback autoplay prob…

4928ca9

…lems in iOS, fixed desync with aresample

Fix last commit

5b07244

read screenshare audio from archived dir insteaf of original path

409e3cb

prlanzarin marked this pull request as draft March 11, 2021 23:48

ffdixon requested a review from kepstin March 12, 2021 01:21

prlanzarin mentioned this pull request Mar 12, 2021

[screenshare] Rewrite SFU/Kurento screensharing bridge (+reconnections|audio sharing|QoL, was #11025) #11622

Merged

antobinary added this to the Release 2.3 milestone Mar 12, 2021

antobinary added the module: recording label Mar 12, 2021

kepstin mentioned this pull request Mar 12, 2021

Perform audio processing encoding with 1 channel (mono-mode) #11633

Open

kepstin reviewed Mar 12, 2021

View reviewed changes

kepstin mentioned this pull request Mar 18, 2021

Recording: Audio processing changes to reduce audio desync #11683

Merged

github-actions bot added the status: conflict label Mar 18, 2021

germanocaumo added 2 commits March 23, 2021 14:47

Merge branch 'develop' of https://github.com/bigbluebutton/bigbluebutton

95fabfe

into u23-recsa

Remove screenshare audio specific ffmpeg codec in recording processing.

136f43e

github-actions bot removed the status: conflict label Mar 23, 2021

kepstin requested changes Mar 24, 2021

View reviewed changes

number of inputs for amix command, remove old java deskshare events

35104fb

prlanzarin marked this pull request as ready for review March 24, 2021 16:55

remove trailing whitespaces

ea375d6

kepstin approved these changes Mar 24, 2021

View reviewed changes

antobinary merged commit a950c9a into bigbluebutton:develop Mar 24, 2021

antobinary mentioned this pull request Mar 24, 2021

Enabled by default screenshare with audio (#11622) #11731

Merged

prlanzarin mentioned this pull request Apr 2, 2021

Share system audio #8632

Closed

prlanzarin deleted the u23-recsa branch October 24, 2021 14:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

screenshare: add rap processing for screen streams with audio (complements #11622) #11626

screenshare: add rap processing for screen streams with audio (complements #11622) #11626

prlanzarin commented Mar 11, 2021 •

edited

kepstin commented Mar 12, 2021

kepstin Mar 12, 2021

prlanzarin Mar 12, 2021

prlanzarin Mar 12, 2021 •

edited

kepstin Mar 12, 2021 •

edited

kepstin Mar 12, 2021

prlanzarin Mar 12, 2021

kepstin Mar 12, 2021

github-actions bot commented Mar 18, 2021

kepstin Mar 24, 2021

kepstin Mar 24, 2021

	ffmpeg_cmd += ['-filter_complex', "amix"]
	ffmpeg_cmd += ['-filter_complex', "amix=inputs=#{inputs.length}"]

screenshare: add rap processing for screen streams with audio (complements #11622) #11626

screenshare: add rap processing for screen streams with audio (complements #11622) #11626

Conversation

prlanzarin commented Mar 11, 2021 • edited

What does this PR do?

Closes Issue(s)

More

kepstin commented Mar 12, 2021

kepstin Mar 12, 2021

Choose a reason for hiding this comment

prlanzarin Mar 12, 2021

Choose a reason for hiding this comment

prlanzarin Mar 12, 2021 • edited

Choose a reason for hiding this comment

kepstin Mar 12, 2021 • edited

Choose a reason for hiding this comment

kepstin Mar 12, 2021

Choose a reason for hiding this comment

prlanzarin Mar 12, 2021

Choose a reason for hiding this comment

kepstin Mar 12, 2021

Choose a reason for hiding this comment

github-actions bot commented Mar 18, 2021

kepstin Mar 24, 2021

Choose a reason for hiding this comment

kepstin Mar 24, 2021

Choose a reason for hiding this comment

prlanzarin commented Mar 11, 2021 •

edited

prlanzarin Mar 12, 2021 •

edited

kepstin Mar 12, 2021 •

edited