1011149 - Video freezes when audio track is disabled on remote stream

Status: VERIFIED FIXED

(Core :: WebRTC: Audio/Video, defect, P2)

Description

[msander]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/34.0.1847.131 Safari/537.36

Steps to reproduce:

1. Open https://apprtc.appspot.com/ in two tabs
2. In on tab call pc.getRemoteStreams()[0].getAudioTracks()[0].enabled=false in the console


Actual results:

Video freezes for the stream with disabled audio.


Expected results:

Video should not freeze when audio is disabled.

Comment 1

[Paul Silaghi, QA [:pauly]]

Confirmed 32.0a1 (2014-05-15), win 7 x64

Comment 2

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Found this patch affect this issue.
# HG changeset patch
# User Paul Adenot <[e-mail]>
# Date 1398431721 -7200
#      Fri Apr 25 15:15:21 2014 +0200
# Node ID 8a69388a1a2116e1b9c3b36e11bd78ba9b98d378
# Parent  27cd1a1fd1d7f8ec0384888f0936fd6c8e1b5f0b
Bug 998711.

Hi Paul, 
Any ideas?

Comment 3

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: patch v1

Fix the mathematical calculation error.

c.mDuration *= aOutRate / aInRate;

mDuration type is int64_t.
In this case, it would divide first and round into integer, then multiplicative mDuration, so we got deviation.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8436707 [details] [diff] [review]
patch v1

Review of attachment 8436707 [details] [diff] [review]:
-----------------------------------------------------------------

Looks right.  Once this lands, please ask for uplift at least to Aurora, and maybe beta as well.

Comment 5

[u459114]

Comment on attachment 8436707 [details] [diff] [review]
patch v1

Review of attachment 8436707 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/AudioSegment.h
@@ +184,5 @@
>        nsAutoTArray<const T*, GUESS_AUDIO_CHANNELS> bufferPtrs;
>        AudioChunk& c = *ci;
>        // If this chunk is null, don't bother resampling, just alter its duration
>        if (c.IsNull()) {
> +        c.mDuration = c.mDuration * aOutRate / aInRate;

Make it more explicit 
c.mDuration = (c.mDuration * aOutRate) / aInRate;

Comment 6

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: patch v1.1

Address cj's comment. I will also request uplift at least to Aurora.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8437402 [details] [diff] [review]
patch v1.1

Review of attachment 8437402 [details] [diff] [review]:
-----------------------------------------------------------------

Let's ask for Aurora and Beta, as beta just opened

Comment 8

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: check-in patch

try result:
https://tbpl.mozilla.org/?tree=Try&rev=5bb39c9cd71c
carry reviewer.

Comment 9

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Request to uplift to other branches.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c953fc808c1f

Friendly reminder that your commit message should be summarizing what the patch is actually doing, not just restating the problem it's fixing. Thanks!
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Committing_Rules_and_Responsibilities#Checkin_comment

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c953fc808c1f

Comment 12

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Comment on attachment 8438114 [details] [diff] [review]
check-in patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 998711
User impact if declined: When user do the webRTC video call, if they disable the remote stream's audio track and that would make video freeze and can't recover.
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky):NA 
String or IDL/UUID changes made by this patch:NA

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Randy, FYI,  tracking-firefox31 is the way to get our attention. Not status-firefox31.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d586e107caa7
https://hg.mozilla.org/releases/mozilla-beta/rev/b3480c1d1f6e

Comment 15

[Petruta Horea [:phorea]]

Reproduced on Nightly 32.0a1 by performing calls between Win 7 64-bit and Mac OSX 10.9.3.

Verified as fixed on Firefox 31 beta 6, latest Aurora 32.0a2 20140702004004 and latest Nightly 33.0a1 20140630030202.