Closed
Bug 1256720
Opened 8 years ago
Closed 8 years ago
Bad assertion in nr_ice_media_stream_check_timer_cb
Categories
(Core :: WebRTC: Networking, defect, P2)
Core
WebRTC: Networking
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: bwc, Unassigned)
Details
Attachments
(1 file)
The if-statement here does not test for whether all streams are complete, just one, so there is no reason to expect that the number of active streams is 0: https://dxr.mozilla.org/mozilla-central/source/media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c#328 What we probably want to do instead is: if (timer_multiplier == 0) timer_multiplier = 1;
Comment 1•8 years ago
|
||
(In reply to Byron Campen [:bwc] from comment #0) > What we probably want to do instead is: > > if (timer_multiplier == 0) timer_multiplier = 1; I guess there is (or should be) no case where active_streams is 0, but ice_state is not COMPLETED and we therefore want the timer_val to be 0.
Updated•8 years ago
|
Rank: 25
Priority: -- → P2
Reporter | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42285/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42285/
Attachment #8734497 -
Flags: review?(drno)
Updated•8 years ago
|
Attachment #8734497 -
Flags: review?(drno) → review+
Comment 3•8 years ago
|
||
Comment on attachment 8734497 [details] MozReview Request: Bug 1256720: Remove a bad assertion, and simplify some code. r=drno https://reviewboard.mozilla.org/r/42285/#review38869 ::: media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c:323 (Diff revision 1) > { > int r,_status; > nr_ice_media_stream *stream=cb_arg; > nr_ice_cand_pair *pair = 0; > - int timer_val; > - int timer_multiplier; > + int timer_multiplier=stream->pctx->active_streams ? stream->pctx->active_streams : 1; > + int timer_val=stream->pctx->ctx->Ta*timer_multiplier; To be on the safe side an assert(timer_val>0) below this or before setting the timer would be nice.
Reporter | ||
Updated•8 years ago
|
Attachment #8734497 -
Attachment description: MozReview Request: Bug 1256720: Remove a bad assertion, and simplify some code. r?drno → MozReview Request: Bug 1256720: Remove a bad assertion, and simplify some code. r=drno
Reporter | ||
Comment 4•8 years ago
|
||
Comment on attachment 8734497 [details] MozReview Request: Bug 1256720: Remove a bad assertion, and simplify some code. r=drno Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42285/diff/1-2/
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/169b38772095
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•