Bug 191490 – RealtimeOutgoing A/V sources should observe their sources only if having a sink

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

This is useless otherwise

[youenn fablet]

Created attachment 354395 [details]
Patch

[youenn fablet]

Created attachment 354413 [details]
Patch

[youenn fablet]

Created attachment 354434 [details]
Patch

[youenn fablet]

Created attachment 354438 [details]
Patch

[Eric Carlson]

Comment on attachment 354438 [details]
Patch

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

> Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.cpp:65
> +    observeSource();

As we discussed, this shouldn't be necessary because you also call it in AddSink.

> Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp:73
> +    observeSource();

Ditto.

> Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp:81
>      m_videoSource->removeObserver(*this);

Nit: why does RealtimeOutgoingAudioSource have unobserveSource and RealtimeOutgoingVideoSource just calls removeObserver?

[youenn fablet]

Thanks for the review.

(In reply to Eric Carlson from comment #5)
> Comment on attachment 354438 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=354438&action=review
> 
> > Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.cpp:65
> > +    observeSource();
> 
> As we discussed, this shouldn't be necessary because you also call it in
> AddSink.

Yes, I will lock the sinks so that we observe/unobserve appropriately.

> > Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp:73
> > +    observeSource();
> 
> Ditto.
> 
> > Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.cpp:81
> >      m_videoSource->removeObserver(*this);
> 
> Nit: why does RealtimeOutgoingAudioSource have unobserveSource and
> RealtimeOutgoingVideoSource just calls removeObserver?

I will add unobserveSource.

[youenn fablet]

Created attachment 354571 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 354571 [details]
Patch for landing

Clearing flags on attachment: 354571

Committed r238102: <https://trac.webkit.org/changeset/238102>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45998054>