New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RtpSender.send() must enforce stricter rules #564

Closed
ibc opened this Issue Jun 7, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@ibc
Contributor

ibc commented Jun 7, 2016

RtpSender.send():

7- For each value of i from 0 to the number of codecs, check that each value of withParameters.codecs[i].name not equal to "red", "rtx" or a forward error correction codec is included in getCapabilities(track.kind).codecs[j].name, where j goes from 0 to the number of codecs. If a match is not found for any value of i, return a promise rejected with InvalidParameters.

That is a too much simplistic check. The fact that the codec name matches one in getCapabilities() does not mean, at all, that it is the "same" codec. Lot of scenarios here:

  • What would happen if send() is called with an "audio/opus" codec that includes rtcpFeedback entries not present in the "audio/opus" codec of the local capabilities?
  • Or what would happen if clockRate does not match?
  • Or imagine that local "video/H264" supports packetization-mode 0 and 1, but send() is called with codec.parameters.packetizationMode: 2.

@aboba aboba changed the title from RtpReceiver.send() must enforce stricter rules to RtpSender.send() must enforce stricter rules Jun 8, 2016

@aboba aboba added the 1.1 label Jun 16, 2016

@aboba

This comment has been minimized.

Contributor

aboba commented Jun 16, 2016

Related to #473

@aboba

This comment has been minimized.

Contributor

aboba commented Oct 5, 2016

Beyond developing the checks, there is also the question of how error reporting works if an issue is found.
Let us say that parameters.headerExtensions[j].uri does not match one of the values of
sender.getCapabilities().headerExtensions[i].uri where i = 0 to the number of header extensions.
Is there text in error.message to make it clear what went wrong?

@ibc

This comment has been minimized.

Contributor

ibc commented Oct 6, 2016

We don't have a granular API here but a send() method that accepts a complex Object params argument. It those params are wrong the method should fail with a TypeError? instance and a custom text message describing the issue. However such a text message is up to each vendor and it's just valid for debugging, nothing else.

aboba added a commit that referenced this issue Oct 9, 2017

Checks for send()
Fix for Issues #564 and #473

@aboba aboba referenced this issue Oct 9, 2017

Merged

Checks for send() #784

@robin-raymond

This comment has been minimized.

Contributor

robin-raymond commented Oct 12, 2017

@ibc did we get everything?

@ibc

This comment has been minimized.

Contributor

ibc commented Oct 16, 2017

Sorry, I was on vacation. Yes, much better now.

@aboba aboba closed this Oct 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment