Skip to content
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

Add "packetsFailedDecryption", to count SRTP decryption failures. #250

Merged
merged 2 commits into from
Oct 23, 2017

Conversation

taylor-b
Copy link
Contributor

Fixes #249.

This could be a useful statistic for diagnosing why calls are failing,
in case an SRTP-related bug is introduced somewhere.

Fixes w3c#249.

This could be a useful statistic for diagnosing why calls are failing,
in case an SRTP-related bug is introduced somewhere.
<p>
The cumulative number of RTP packets that failed to be decrypted according
to the procedures in [[!RFC3711]]. These packets are not counted by
<code><a>packetsReceived</a></code> or <code><a>packetsDiscarded</a></code>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit odd, packetsReceived counter should go up when a packet is received and not after they have been decrypted. Aren't the packetsSent incremented when the frames are packetized and not when they are encrypted.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace last sentence with "These packets are not counted by packetsDiscarded."

@vr000m
Copy link
Contributor

vr000m commented Oct 20, 2017

RFC3711 states:

Conceptually, we consider SRTP to be a "bump in the stack"
implementation which resides between the RTP application and the
transport layer.

So I was incorrect, this means that packetsReceived and packetsDiscarded do not increment when a packet fails decryption.

@alvestrand alvestrand merged commit af2e9c7 into w3c:master Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants