[webkit-reviews] review granted: [Bug 204290] Introduce a mock implementation of CoreAudioSharedUnit : [Attachment 383829] 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 204290: Introduce a mock implementation of CoreAudioSharedUnit
https://bugs.webkit.org/show_bug.cgi?id=204290

Attachment 383829: Patch

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




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

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

> Source/WebCore/platform/mediastream/mac/BaseAudioSharedUnit.h:71
> +    virtual CapabilityValueOrRange sampleRateCapacities() const { return
CapabilityValueOrRange(8000, 96000); }

Nit: it is a bit strange to have this return a default value from the base
class when other trivial methods (isProducingData, hasAudioUnit) do not. It
would be cleaner to also make it pure virtual.

> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:896
> +    auto& unit = m_overrideUnit ? *m_overrideUnit :
CoreAudioSharedUnit::singleton();
>      if (unit.isSuspended())

if (this->unit().isSuspended())

> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:905
> +    if (m_overrideUnit)
> +	   m_overrideUnit->delaySamples(seconds);

Maybe have the non-mock unit, or the base class, have an empty delaySamples
method instead?