Find all matching SDP codecs for defaults #530
Conversation
src/web_app/js/sdputils.js
Outdated
| var payload = null; | ||
| for (var i = 0; i < sdpLines.length; i++) { | ||
| var index = findLineInRange(sdpLines, i, -1, 'a=rtpmap', codec); | ||
| if(index !== null) { |
There was a problem hiding this comment.
Space between if and (.
tylergibson
force-pushed
the
fix-multi-sdp-h264
branch
from
19339ae to
f161c71
Compare
|
@KaptenJansson Thanks, fixed the spacing nit and amended the commit. Also I believe this resolves #492, as it prioritizes all h264 profiles and will cycle offer candidates until one is accepted. |
src/web_app/js/sdputils.js
Outdated
| payload = getCodecPayloadTypeFromLine(sdpLines[index]); | ||
| if (payload) { | ||
| sdpLines[mLineIndex] = setDefaultCodec(sdpLines[mLineIndex], payload); | ||
| } |
There was a problem hiding this comment.
SDP lines from i to index was scanned in findLineInRange. Perhaps we can skip them by adding i = index; here.
There was a problem hiding this comment.
@jianjunz I originally had exactly that, but removed it thinking it was redundant. You're right though it would skip the next index - i iterations. I'll update
|
Thanks to @jianjunz, added the index to i assignment to reduce loop iterations. Also added null break to exit the search once no match is found. Finally, updated findLineInRange to search ascending or descending. By doing a descending search we end up with all matched codecs in the same preference order as the original SDP, preserving client preference. |
tylergibson
force-pushed
the
fix-multi-sdp-h264
branch
8 times, most recently
from
94c5051 to
616cdac
Compare
tylergibson
force-pushed
the
fix-multi-sdp-h264
branch
from
616cdac to
5ef563d
Compare
|
@KaptenJansson Pingback, anything blocking accepting this PR? |
|
Will publish a new AppRTC release later this week. |
Small fix for setting H264 and other multi-profile codecs as default for SDP offers.