Closed Bug 1167306 Opened 4 years ago Closed 4 years ago

Several webrtc prefs are broken

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: bwc, Assigned: bwc)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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.
What were you shooting for with this change?

#if defined(MOZILLA_INTERNAL_API) && !defined(MOZILLA_XPCOMRT_API) ?
Flags: needinfo?(rbarker)
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: nobody → docfaraday
Attached file MozReview Request: bz://1167306/bwc (obsolete) —
/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/
Attachment #8608977 - Flags: review?(rjesup)
Attachment #8608977 - Flags: review?(rjesup) → review+
We need to uplift this to 40 as well
Flags: needinfo?(rjesup)
Rank: 10
Priority: P2 → P1
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
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Attachment #8608977 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8608977 - Attachment is obsolete: true
Attachment #8620350 - Flags: review+
You need to log in before you can comment on or make changes to this bug.