Bug 185545 – Add release logging for incoming and outgoing webrtc audio tracks

• Component: WebRTC
• Status: RESOLVED FIXED

[youenn fablet]

Add release logging for incoming and outgoing webrtc audio tracks

[youenn fablet]

Created attachment 340185 [details]
Patch

[youenn fablet]

Created attachment 340186 [details]
Patch

[Daniel Bates]

Comment on attachment 340186 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        one log line is added each second for each audio track.

What is the motivation for such logging? Is this to diagnosis a hard to find bug? When can we remove this logging?  I am not at a computer with a checkout at the moment, is this logging always enabled?

[Daniel Bates]

Comment on attachment 340186 [details]
Patch

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

>> Source/WebCore/ChangeLog:9
>> +        one log line is added each second for each audio track.
> 
> What is the motivation for such logging? Is this to diagnosis a hard to find bug? When can we remove this logging?  I am not at a computer with a checkout at the moment, is this logging always enabled?

Is it necessary to to log every second? That seems pretty frequent.

[youenn fablet]

Logging is always enabled and allows verifying that audio capture is working fine.
We have the same logging for video capture.

There are cases where a webrtc call fails.
This logging allows to check whether capture is working fine or not.
Our current way of diagnose this is to ask users to turn mock capture sources and retry.

It is not necessary to log every second.
We need a value that is smaller than the timeout values some websites use for error handling.

[youenn fablet]

Ping review.
This would help investigating some cases where audio is not sent as can be seen from stats found in rdar://problem/37516414.

[Eric Carlson]

Comment on attachment 340186 [details]
Patch

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

> Source/WebCore/platform/mediastream/mac/RealtimeIncomingAudioSourceCocoa.cpp:78
> +    if (!(++m_chunksReceived % 100))

Nit: it would be useful to log a unique identifier to help debug cases with more than one track.

> Source/WebCore/platform/mediastream/mac/RealtimeOutgoingAudioSourceCocoa.cpp:161
> +            RELEASE_LOG(MediaStream, "RealtimeOutgoingAudioSourceCocoa::pullAudioData %zu chunk", m_chunksSent);

Ditto

> Source/WebKit/WebProcess/cocoa/UserMediaCaptureManager.cpp:237
> +        RELEASE_LOG(MediaStream, "UserMediaCaptureManager::audioSamplesAvailable %zu chunk for %llu", m_receivedSamples, id);

Ditto

[youenn fablet]

Created attachment 359311 [details]
Patch for landing

[youenn fablet]

(In reply to Eric Carlson from comment #7)
> Comment on attachment 340186 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=340186&action=review
> 
> > Source/WebCore/platform/mediastream/mac/RealtimeIncomingAudioSourceCocoa.cpp:78
> > +    if (!(++m_chunksReceived % 100))
> 
> Nit: it would be useful to log a unique identifier to help debug cases with
> more than one track.

Good idea.
This makes it a larger patch with more refactoring and I applied it to existing sources as well.
I plan to land it tomorrow, if you want to have a quick look before, do not hesitate.

[youenn fablet]

Created attachment 359312 [details]
Patch for landing

[Eric Carlson]

(In reply to youenn fablet from comment #9)
> 
> Good idea.
> This makes it a larger patch with more refactoring and I applied it to
> existing sources as well.
> I plan to land it tomorrow, if you want to have a quick look before, do not
> hesitate.

Nice improvement, thanks!

[youenn fablet]

Created attachment 359374 [details]
Fix GTK

[WebKit Commit Bot]

Comment on attachment 359374 [details]
Fix GTK

Clearing flags on attachment: 359374

Committed r240120: <https://trac.webkit.org/changeset/240120>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/47357698>