Several webrtc prefs are broken

RESOLVED FIXED in Firefox 40

Status

()

Core
WebRTC: Audio/Video
P1
normal
Rank:
10
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bwc, Assigned: bwc)

Tracking

({regression})

Trunk
mozilla41
regression
Points:
---

Firefox Tracking Flags

(firefox40 fixed, firefox41 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
We've disabled a good chunk of code in WebrtcVideoConduit::Init() with a preprocessor goof:

https://dxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp#281

This breaks:

media.video.test_latency
media.peerconnection.video.min_bitrate
media.peerconnection.video.start_bitrate
media.peerconnection.video.max_bitrate
media.navigator.load_adapt

This also means that the load manager is disabled.
(Assignee)

Comment 1

3 years ago
What were you shooting for with this change?

#if defined(MOZILLA_INTERNAL_API) && !defined(MOZILLA_XPCOMRT_API) ?
Flags: needinfo?(rbarker)
(Assignee)

Comment 2

3 years ago
P2 sound about right here?
Flags: needinfo?(rjesup)
Priority: -- → P2
(In reply to Byron Campen [:bwc] from comment #1)
> What were you shooting for with this change?
> 
> #if defined(MOZILLA_INTERNAL_API) && !defined(MOZILLA_XPCOMRT_API) ?

Yes.
Flags: needinfo?(rbarker)
(Assignee)

Updated

3 years ago
Assignee: nobody → docfaraday
(Assignee)

Comment 4

3 years ago
Created attachment 8608977 [details]
MozReview Request: bz://1167306/bwc

/r/9207 - Bug 1167306: Fix preprocessor goof that disabled the load manager and some preference handling.

Pull down this commit:

hg pull -r d58ed278d8389cbbfc1de985722eb20fff6b2b71 https://reviewboard-hg.mozilla.org/gecko/
(Assignee)

Updated

3 years ago
Attachment #8608977 - Flags: review?(rjesup)

Updated

3 years ago
Attachment #8608977 - Flags: review?(rjesup) → review+
Comment on attachment 8608977 [details]
MozReview Request: bz://1167306/bwc

https://reviewboard.mozilla.org/r/9205/#review7907

Ship It!
We need to uplift this to 40 as well
status-firefox40: --- → affected
Flags: needinfo?(rjesup)

Updated

3 years ago
Rank: 10
Priority: P2 → P1

Comment 7

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c3d1cb210da
(Assignee)

Comment 8

3 years ago
Comment on attachment 8608977 [details]
MozReview Request: bz://1167306/bwc

Approval Request Comment
[Feature/regressing bug #]:

   Bug 1101651

[User impact if declined]:

   Poor webrtc audio/video on devices that are performance constrained, since the load manager is disabled.

[Describe test coverage new/current, TreeHerder]:

   Unfortunately load adaptation is not tested at all right now.

[Risks and why]: 

   Extremely low; the only real risk here is discovering some regression in the load adaptation code that went unnoticed because it was disabled.

[String/UUID change made/needed]:

   None.
Attachment #8608977 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/5c3d1cb210da
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Attachment #8608977 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/f13fdaa24c37
status-firefox40: affected → fixed
(Assignee)

Comment 11

3 years ago
Comment on attachment 8608977 [details]
MozReview Request: bz://1167306/bwc
Attachment #8608977 - Attachment is obsolete: true
Attachment #8620350 - Flags: review+
(Assignee)

Comment 12

3 years ago
Created attachment 8620350 [details]
MozReview Request: Bug 1167306: Fix preprocessor goof that disabled the load manager and some preference handling.
You need to log in before you can comment on or make changes to this bug.