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

test: add rtcp_mux test #2692

Merged
merged 2 commits into from
Aug 30, 2023
Merged

test: add rtcp_mux test #2692

merged 2 commits into from
Aug 30, 2023

Conversation

cspiel1
Copy link
Collaborator

@cspiel1 cspiel1 commented Aug 30, 2023

  • test: add rtcp_mux test
  • test: rtcp test wait for RTCP SR instead off RTCP APP

@cspiel1 cspiel1 marked this pull request as draft August 30, 2023 09:31
@cspiel1
Copy link
Collaborator Author

cspiel1 commented Aug 30, 2023

The RTCP_APP packet is received in stream.c rtp_handler() because the rs->rtcp_mux in rtp.c was not set fast enough.

@cspiel1
Copy link
Collaborator Author

cspiel1 commented Aug 30, 2023

stream->rtcp_mux is set if SDP media remote attribute rtcp-mux was set. Thus it most likely will be set too late. I have no clue how to fix this.

@cspiel1
Copy link
Collaborator Author

cspiel1 commented Aug 30, 2023

So currently the test stops if an RCTP APP packet is received. Which I guess is only meant to open a NAT pinhole for RTCP. Do we really need the RTCP APP at the receiver at the very beginning of the call?

Shouldn't we prefer to test if an RTCP SR is received?
For that we have to specify an audio source. Otherwise no RTP packets are sent and no RTP tx stat is available.

I will add an audio source and try to activate the RTCP SR. This will fix the RTCP mux test.

@cspiel1
Copy link
Collaborator Author

cspiel1 commented Aug 30, 2023

updated the description

@sreimers : pls could we first merge this?
Then I'll rebase rx_thread_1 and rx_thread_stream_receiver. With the later I'll repeat the sanitizer checks you mentioned.

@cspiel1 cspiel1 marked this pull request as ready for review August 30, 2023 12:57
@sreimers sreimers merged commit ffc493c into baresip:main Aug 30, 2023
17 checks passed
@cspiel1 cspiel1 deleted the test_rtcp_mux branch August 30, 2023 13:09
@cspiel1
Copy link
Collaborator Author

cspiel1 commented Aug 30, 2023

Thanks! This would be another PR for test_call_rtcp_base(): #2687
I have a series of commits that finally adds more tests for RX thread. Do you have time for review?

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.

None yet

2 participants