Typos in vcmCheckAttribs

RESOLVED FIXED in mozilla35

Status

()

Core
WebRTC: Audio/Video
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bwc, Assigned: bwc)

Tracking

Trunk
mozilla35
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
We're fetching the wrong params here for max_dpb and max_fs:

        if ( ccsdpAttrGetFmtpMaxCpb(sdp_p, level, 0, fmtp_inst, &t_uint) == SDP_SUCCESS )
        {
            rcap->max_cpb = t_uint;
        }

        if ( ccsdpAttrGetFmtpMaxCpb(sdp_p, level, 0, fmtp_inst, &t_uint) == SDP_SUCCESS )
        {
            rcap->max_dpb = t_uint;
        }

        if ( ccsdpAttrGetFmtpMaxCpb(sdp_p, level, 0, fmtp_inst, &t_uint) == SDP_SUCCESS )
        {
            rcap->max_br = t_uint;
        }
(Assignee)

Comment 1

3 years ago
Created attachment 8496306 [details] [diff] [review]
Fix typos in vcmCheckAttribs that caused the max-dpb and max-fr to be set equal to the max-cpb

Should be simple enough.
(Assignee)

Updated

3 years ago
Assignee: nobody → docfaraday
Status: NEW → ASSIGNED
(Assignee)

Comment 2

3 years ago
Comment on attachment 8496306 [details] [diff] [review]
Fix typos in vcmCheckAttribs that caused the max-dpb and max-fr to be set equal to the max-cpb

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

https://tbpl.mozilla.org/?tree=Try&rev=606272c3e3b5
Attachment #8496306 - Flags: review?(rjesup)

Updated

3 years ago
Attachment #8496306 - Flags: review?(rjesup) → review+
(In reply to Byron Campen [:bwc] from comment #1)
> Created attachment 8496306 [details] [diff] [review]
> Fix typos in vcmCheckAttribs that caused the max-dpb and max-fr to be set
> equal to the max-cpb

Actually, it was just doing tell-me-three-times on cpb, and letting dpb and br default.
(Assignee)

Comment 4

3 years ago
I'm pretty sure we actually were setting all three to the same thing, since we were re-using t_uint in each case. Should we double-check with some H264 tests to be sure this doesn't have any surprising side-effects?
(In reply to Byron Campen [:bwc] from comment #4)
> I'm pretty sure we actually were setting all three to the same thing, since
> we were re-using t_uint in each case. Should we double-check with some H264
> tests to be sure this doesn't have any surprising side-effects?

never mind, you're right.

max-br would be good to check; set it and make a call and check network traffic.
(Assignee)

Comment 6

3 years ago
So I've tested the patch without tweaking any of these params, and I'm not seeing any deleterious effects. I'll give a try and see what happens when I set the max_br to something smallish.
(Assignee)

Comment 7

3 years ago
max_br does seem to limit the bandwidth used.
(Assignee)

Comment 8

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a04cf72848f
https://hg.mozilla.org/mozilla-central/rev/4a04cf72848f
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.