Bug 174456 - WebRTC: silence data not sent for disabled audio track
Summary: WebRTC: silence data not sent for disabled audio track
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-07-12 22:15 PDT by Andrew Morris
Modified: 2017-08-30 18:14 PDT (History)
7 users (show)

See Also:


Attachments
Patch (12.12 KB, patch)
2017-07-13 09:28 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews116 for mac-elcapitan (1.72 MB, application/zip)
2017-07-13 10:52 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (963.25 KB, application/zip)
2017-07-13 10:56 PDT, Build Bot
no flags Details
Patch (12.17 KB, patch)
2017-07-14 11:37 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Morris 2017-07-12 22:15:04 PDT
jsbin: https://output.jsbin.com/doweto

Press start, call

Output:
{
  "pollIndex": 71,
  "audioBytesSent": 0,
  "videoBytesSent": 7412286
}

Expected output:
{
  "pollIndex": 71,
  "audioBytesSent": <not zero>, // <--- difference here
  "videoBytesSent": 7412286
}

---

If you disable video instead, a little video data is still sent for the black frames. In a similar way, when audio is disabled, a little silence data should be sent too. At tokbox, we use these data to ensure the stream is working properly at the SFU and will reject the stream if video/audio is disabled during startup (if it is set from the very start it will accept it). The spec says this silence data should be sent:

"If track is ended, or if track.muted is set to true, the RTCRtpSender sends silence (audio) or a black frame (video). If track is set to null then the RTCRtpSender does not send."

Chrome and FF have this behaviour using the jsbin provided.
Comment 1 Radar WebKit Bug Importer 2017-07-12 23:55:41 PDT
<rdar://problem/33284623>
Comment 2 youenn fablet 2017-07-13 09:28:27 PDT
Created attachment 315350 [details]
Patch
Comment 3 Eric Carlson 2017-07-13 10:45:17 PDT
Comment on attachment 315350 [details]
Patch

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

> Source/WebCore/platform/mediastream/mac/RealtimeOutgoingAudioSource.cpp:95
> +    bool isMuted = m_muted || !m_enabled;

Minor nit: "isMuted" is slightly misleading, maybe "isSilenced" or "sendSilence"?
Comment 4 Build Bot 2017-07-13 10:52:16 PDT
Comment on attachment 315350 [details]
Patch

Attachment 315350 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4113907

New failing tests:
storage/indexeddb/modern/new-database-after-user-delete.html
Comment 5 Build Bot 2017-07-13 10:52:17 PDT
Created attachment 315355 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 6 Build Bot 2017-07-13 10:56:01 PDT
Comment on attachment 315350 [details]
Patch

Attachment 315350 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4113915

New failing tests:
imported/w3c/IndexedDB-private-browsing/idbfactory_open12.html
Comment 7 Build Bot 2017-07-13 10:56:03 PDT
Created attachment 315357 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 8 youenn fablet 2017-07-13 15:14:14 PDT
Both tests are indexed db tests and are not impacted by this change.
Comment 9 youenn fablet 2017-07-14 11:37:05 PDT
Created attachment 315465 [details]
Patch
Comment 10 WebKit Commit Bot 2017-07-14 14:43:39 PDT
Comment on attachment 315465 [details]
Patch

Clearing flags on attachment: 315465

Committed r219524: <http://trac.webkit.org/changeset/219524>
Comment 11 WebKit Commit Bot 2017-07-14 14:43:40 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Aiham Hammami 2017-08-30 18:14:48 PDT
Is this fix included in Safari Tech Preview Release 38?

I'm still seeing this behaviour in Safari Tech Preview Release 38 (Safari 11.1, WebKit 12605.1.3.1).

1. Navigate to https://output.jsbin.com/doweto
2. Ensure Audio is unchecked, and Video is checked
3. Press Start, then Call

Actual Output:
{
  "pollIndex": 117,
  "audioBytesSent": 0,
  "videoBytesSent": 12122541
}


Expected Output:
{
  "pollIndex": 117,
  "audioBytesSent": <not zero>,
  "videoBytesSent": 12122541
}

This works as expected when Audio is checked and Video is unchecked.