[webkit-reviews] review granted: [Bug 211718] Introduce a RealtimeMediaSource video sample observer : [Attachment 399017] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 211718: Introduce a RealtimeMediaSource video sample observer
https://bugs.webkit.org/show_bug.cgi?id=211718

Attachment 399017: Patch

https://bugs.webkit.org/attachment.cgi?id=399017&action=review




--- Comment #3 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 399017
  --> https://bugs.webkit.org/attachment.cgi?id=399017
Patch

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

> Source/WebCore/platform/mediarecorder/MediaRecorderPrivate.h:107
>      if (m_audioSource)
>	   m_audioSource->removeAudioSampleObserver(*this);
> +    if (m_videoSource)
> +	   m_videoSource->removeVideoSampleObserver(*this);

Maybe add a FIXME about removing these, since they should never be necessary?

> Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:210
> +    auto locker = holdLock(m_videoSampleObserversLock);
> +    for (auto* observer : m_videoSampleObservers)
> +	   observer->videoSampleAvailable(mediaSample);

Clients may perform an arbitrary amount of processing during a callback, so
holding the lock for the duration of this loop might be asking for trouble.