Add release logging for incoming and outgoing webrtc audio tracks
Created attachment 340185 [details] Patch
Created attachment 340186 [details] Patch
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?
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.
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.
Ping review. This would help investigating some cases where audio is not sent as can be seen from stats found in rdar://problem/37516414.
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
Created attachment 359311 [details] Patch for landing
(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.
Created attachment 359312 [details] Patch for landing
(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!
Created attachment 359374 [details] Fix GTK
Comment on attachment 359374 [details] Fix GTK Clearing flags on attachment: 359374 Committed r240120: <https://trac.webkit.org/changeset/240120>
All reviewed patches have been landed. Closing bug.
<rdar://problem/47357698>