Bug 169727 – Improve WebRTC track enabled support

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[youenn fablet]

These tests could be improved a bit and made more clear why they fail.

[youenn fablet]

Created attachment 304607 [details]
Patch

[youenn fablet]

This is a first step.
If we follow that direction, we might want to handle that at RealtimeMediaSource level, so that it generates the black frame/silence in videoSampleAvailable/audioSamplesAvailable

[youenn fablet]

Created attachment 304608 [details]
Patch

[Alex Christensen]

Comment on attachment 304608 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        Mkaking sure muted/disabled sources produce silence/black frames.

Making

> Source/WebCore/ChangeLog:24
> +        (WebCore::RealtimeIncomingVideoSource::pixelBufferFromVideoFrame):i Generating black frames if muted.

i

> Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSource.cpp:100
> +            auto status = CVPixelBufferCreate(kCFAllocatorDefault, frame.width(), frame.height(), kCVPixelFormatType_420YpCbCr8Planar, nullptr, &pixelBuffer);

This looks like a memory leak.  What deallocates this?  We should probably return a smart pointer type.

> Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSource.cpp:104
> +            ASSERT(!status);

Does status of 0 have a name?

[youenn fablet]

Created attachment 304641 [details]
Patch for landing

[youenn fablet]

Thanks for the review

(In reply to comment #4)
> Comment on attachment 304608 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=304608&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        Mkaking sure muted/disabled sources produce silence/black frames.
> 
> Making

OK

> 
> > Source/WebCore/ChangeLog:24
> > +        (WebCore::RealtimeIncomingVideoSource::pixelBufferFromVideoFrame):i Generating black frames if muted.
> 
> i

OK

> 
> > Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSource.cpp:100
> > +            auto status = CVPixelBufferCreate(kCFAllocatorDefault, frame.width(), frame.height(), kCVPixelFormatType_420YpCbCr8Planar, nullptr, &pixelBuffer);
> 
> This looks like a memory leak.  What deallocates this?  We should probably
> return a smart pointer type.

pixelBuffer is retained by m_blackFrame.
I put the assignment closer to the allocation.


> 
> > Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSource.cpp:104
> > +            ASSERT(!status);
> 
> Does status of 0 have a name?

Changed to noErr.

[WebKit Commit Bot]

Comment on attachment 304641 [details]
Patch for landing

Clearing flags on attachment: 304641

Committed r214044: <http://trac.webkit.org/changeset/214044>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.