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

RTC::Consumer::GetStats() | failed assertion `it != this->mapProfileRtpStream.end()': effective profile does not map to a rtp stream #196

Closed
ibc opened this issue Mar 26, 2018 · 10 comments
Assignees
Labels
Milestone

Comments

@ibc
Copy link
Member

ibc commented Mar 26, 2018

2018-03-23T11:58:08.343Z mediasoup:WARN:mediasoup-worker [id:lemrotac#1] RTC::RtpPacket::Parse() | number of padding octets is greater than available space for payload, packet discarded
2018-03-23T11:58:08.343Z mediasoup:WARN:mediasoup-worker [id:lemrotac#1] RTC::WebRtcTransport::OnRtpDataRecv() | DecryptSrtp() failed due to an invalid RTP packet
2018-03-23T11:58:08.348Z mediasoup:WARN:mediasoup-worker [id:lemrotac#1] RTC::WebRtcTransport::OnRtpDataRecv() | DecryptSrtp() failed [ssrc:3466239622, payloadType:111, seq:10252]
2018-03-23T11:58:08.351Z mediasoup:WARN:mediasoup-worker [id:lemrotac#1] RTC::WebRtcTransport::OnRtpDataRecv() | DecryptSrtp() failed [ssrc:4179093344, payloadType:97, seq:35880]
2018-03-23T11:58:08.351Z mediasoup:WARN:mediasoup-worker [id:lemrotac#1] RTC::WebRtcTransport::OnRtpDataRecv() | DecryptSrtp() failed [ssrc:4179093344, payloadType:97, seq:35881]
2018-03-23T11:58:08.353Z mediasoup:WARN:mediasoup-worker [id:lemrotac#1] RTC::WebRtcTransport::OnRtpDataRecv() | DecryptSrtp() failed [ssrc:4179093344, payloadType:97, seq:35882]
2018-03-23T11:58:08.360Z mediasoup:WARN:mediasoup-worker [id:lemrotac#1] RTC::WebRtcTransport::OnRtpDataRecv() | DecryptSrtp() failed [ssrc:3466239622, payloadType:111, seq:10253]
2018-03-23T11:58:08.360Z mediasoup:WARN:mediasoup-worker [id:lemrotac#1] RTC::WebRtcTransport::OnRtpDataRecv() | DecryptSrtp() failed [ssrc:4179093344, payloadType:97, seq:35883]
mediasoup-worker's stderr: ABORT[id:lemrotac#1] RTC::Consumer::GetStats() | failed assertion `it != this->mapProfileRtpStream.end()': effective profile does not map to a rtp stream
2018-03-23T11:58:08.368Z mediasoup:ERROR:Worker child process exited [id:lemrotac#1, code:null, signal:SIGABRT]

I don't think the initial logs have any relationship.

@ibc ibc added the bug label Mar 26, 2018
@ibc ibc assigned ibc and jmillan Mar 26, 2018
@ibc
Copy link
Member Author

ibc commented Aug 17, 2018

Ping @jmillan

jmillan added a commit that referenced this issue Aug 18, 2018
Scenario:

`Consumer::RemoveProfile()` is called with the profile pointed by `targetProfile`.
The corresponding entry is erased from `mapProfileRtpStream` container.

`targetProfile` is not being cleared, and here resides the bug.

Few RTP packets later a packet with the profile pointed by `targetProfile` comes
in `SendRtpPacket()`. We were waiting for it, so we make such profile the
`effectiveProfile`. At this point `effectiveProfile` points to a profile which
is not present in `mapProfileRtpStream` and this crash will happen on the next
call to `GetStats()`.
@ibc
Copy link
Member Author

ibc commented Aug 20, 2018

So fixed in 6508861

@ibc ibc closed this as completed Aug 20, 2018
@ibc
Copy link
Member Author

ibc commented Oct 15, 2018

It happens again when testing H264 simulcast:

mediasoup-worker's stderr: ABORT[id:zppauszk#4] RTC::Consumer::GetStats() | failed assertion `it != this->mapProfileRtpStream.end()': effective profile does not map to a rtp stream

@ibc ibc reopened this Oct 15, 2018
@ibc ibc added this to the v2 updates milestone Oct 15, 2018
@ibc
Copy link
Member Author

ibc commented Oct 15, 2018

@jmillan this happened testing Safari Tech locally. My Safari is somehow tired, it sends H264 with poor quality, so the remote Consumer (the one that receives Safari's H264 and the one who enabled getStats()) is most of the time with effective profile "none" and so on.

@ibc
Copy link
Member Author

ibc commented Oct 16, 2018

What I see is:

  • this->mapProfileRtpStream has "low" and "medium".
  • this->effectiveProfile is "medium".
  • Consumer::RemoveProfile("medium") is called.
    • this->mapProfileRtpStream now has just "low".

BUT:

this code in RemoveProfile():

		// There is an ongoing probation for this profile, disable it.
		if (IsProbing() && this->probingProfile == profile)
		{
			// Stop probation.
			StopProbation();

			// Reset the RTP monitor so this probation doesn't affect the current profile.
			this->rtpMonitor->Reset();

			return;
		}

		if (this->effectiveProfile == profile)
		{
			SetEffectiveProfile(RtpEncodingParameters::Profile::NONE);
		}
		else if (this->targetProfile == profile)
		{
			this->targetProfile = this->effectiveProfile;
		}
  • If this happens: if (IsProbing() && this->probingProfile == profile) then the method returns.
  • So SetEffectiveProfile(RtpEncodingParameters::Profile::NONE); is not called.
  • And this->effectiveProfile remains being "medium".
  • But "medium" is no longer in this-> mapProfileRtpStream.
  • So crash.

@ibc ibc closed this as completed in b684041 Oct 16, 2018
@ibc ibc reopened this Oct 16, 2018
@ibc
Copy link
Member Author

ibc commented Oct 16, 2018

It may be fixed in e96379d. I've just removed a return; line (which IMHO does not make any sense).

@ibc ibc closed this as completed in e96379d Oct 16, 2018
@ibc ibc reopened this Oct 16, 2018
@ibc
Copy link
Member Author

ibc commented Oct 16, 2018

(sorry for the double commit, git amend plus git push --force did not work as I expected).

@jmillan
Copy link
Member

jmillan commented Oct 16, 2018

👍 Good catch

@ibc
Copy link
Member Author

ibc commented Oct 16, 2018

The problem is that, when we call setEffectiveProfile() we don't check if the probingProfile matches the new effective profile so, in that case, both effective and probing profiles are the same, producing the crash that has been worked around by removing the return in RemoveStream().

@ibc
Copy link
Member Author

ibc commented Oct 16, 2018

Let's close this until a refactor is done.

@ibc ibc closed this as completed Oct 16, 2018
lavarsicious pushed a commit to lavarsicious/mediasoup that referenced this issue Feb 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants