Bug 210180

Summary: Introduce a RealtimeMediaSource observer dedicated to receiving audio samples
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebRTCAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, eric.carlson, ews-watchlist, glenn, hta, jacob_uphoff, japhet, jer.noble, philipj, sergio, tommyw, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing
none
Patch for landing
none
Patch for landing none

Description youenn fablet 2020-04-08 03:42:17 PDT 
Introduce a RealtimeMediaSource observer dedicated to receiving audio samples
Comment 1 youenn fablet 2020-04-08 04:09:30 PDT 
Created attachment 395788 [details]
Patch
Comment 2 youenn fablet 2020-04-08 04:55:34 PDT 
Created attachment 395792 [details]
Patch
Comment 3 EWS 2020-04-08 08:37:52 PDT 
ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.
Comment 4 Eric Carlson 2020-04-08 10:11:31 PDT 
Comment on attachment 395792 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395792&action=review

> Source/WebCore/Modules/mediarecorder/MediaRecorder.h:41
> +typedef std::unique_ptr<MediaRecorderPrivate>(*creatorFunction)(MediaStreamPrivate&);

Nit: could change to "using"

> Source/WebCore/platform/mediastream/AudioTrackPrivateMediaStream.cpp:126
> +    if (!m_shouldPlay || m_muted || streamTrack().muted() || streamTrack().ended() || !streamTrack().enabled()) {
> +        stopRenderer();
>          return;

Might as well stop the renderer when the volume is set to 0 too.

> Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.h:125
> +    void audioSamplesAvailable(const MediaTime&, const PlatformAudioData&, const AudioStreamDescription&, size_t) override { };

As we discussed, it may be possible to remove this.
Comment 5 youenn fablet 2020-04-09 08:19:11 PDT 
(In reply to Eric Carlson from comment #4)
> Comment on attachment 395792 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=395792&action=review
> 
> > Source/WebCore/Modules/mediarecorder/MediaRecorder.h:41
> > +typedef std::unique_ptr<MediaRecorderPrivate>(*creatorFunction)(MediaStreamPrivate&);
> 
> Nit: could change to "using"

OK.

> > Source/WebCore/platform/mediastream/AudioTrackPrivateMediaStream.cpp:126
> > +    if (!m_shouldPlay || m_muted || streamTrack().muted() || streamTrack().ended() || !streamTrack().enabled()) {
> > +        stopRenderer();
> >          return;
> 
> Might as well stop the renderer when the volume is set to 0 too.

Right, will add a call to updateRenderer ion setVolume.

> > Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.h:125
> > +    void audioSamplesAvailable(const MediaTime&, const PlatformAudioData&, const AudioStreamDescription&, size_t) override { };
> 
> As we discussed, it may be possible to remove this.

OK
Comment 6 youenn fablet 2020-04-09 08:25:18 PDT 
Created attachment 395953 [details]
Patch for landing
Comment 7 youenn fablet 2020-04-09 09:24:52 PDT 
Created attachment 395961 [details]
Patch for landing
Comment 8 EWS 2020-04-09 12:44:27 PDT 
Committed r259816: <https://trac.webkit.org/changeset/259816>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395961 [details].
Comment 9 Radar WebKit Bug Importer 2020-04-09 12:45:14 PDT 
<rdar://problem/61533569>
Comment 10 Jacob Uphoff 2020-04-09 13:49:15 PDT 
Reverted r259816 for reason:

This commit broke the webkit build for macOS and iOS

Committed r259824: <https://trac.webkit.org/changeset/259824>
Comment 11 youenn fablet 2020-04-09 23:49:00 PDT 
Created attachment 396052 [details]
Patch for landing
Comment 12 EWS 2020-04-10 03:58:46 PDT 
Committed r259861: <https://trac.webkit.org/changeset/259861>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396052 [details].