Introduce a RealtimeMediaSource observer dedicated to receiving audio samples

[youenn fablet]

Introduce a RealtimeMediaSource observer dedicated to receiving audio samples

[youenn fablet]

Created attachment 395788 [details]
Patch

[youenn fablet]

Created attachment 395792 [details]
Patch

[EWS]

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

[Eric Carlson]

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.

[youenn fablet]

(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

[youenn fablet]

Created attachment 395953 [details]
Patch for landing

[youenn fablet]

Created attachment 395961 [details]
Patch for landing

[EWS]

Committed r259816: <https://trac.webkit.org/changeset/259816>

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

[Radar WebKit Bug Importer]

<rdar://problem/61533569>

[Jacob Uphoff]

Reverted r259816 for reason:

This commit broke the webkit build for macOS and iOS

Committed r259824: <https://trac.webkit.org/changeset/259824>

[youenn fablet]

Created attachment 396052 [details]
Patch for landing

[EWS]

Committed r259861: <https://trac.webkit.org/changeset/259861>

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