Closed Bug 1073710 Opened 7 years ago Closed 7 years ago

Typos in vcmCheckAttribs

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: bwc, Assigned: bwc)

Details

Attachments

(1 file)

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: nobody → docfaraday
Status: NEW → ASSIGNED
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)
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.
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.
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.
max_br does seem to limit the bandwidth used.
https://hg.mozilla.org/mozilla-central/rev/4a04cf72848f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.