search

awslabs / amazon-kinesis-video-streams-webrtc-sdk-c

Amazon Kinesis Video Streams Webrtc SDK is for developers to install and customize realtime communication between devices and enable secure streaming of video, audio to Kinesis Video Streams.
https://awslabs.github.io/amazon-kinesis-video-streams-webrtc-sdk-c/group__PublicMemberFunctions.html
Apache License 2.0
1.01k stars 304 forks source link

[FEATURE] Parse candidate type of remote candidates in iceAgentAddRemoteCandidate #1031

Closed yuma-m closed 3 years ago

yuma-m commented 3 years ago

Is your feature request related to a problem? Please describe. Related to #1030, candidate type of remote candidates is not parsed in iceAgentAddRemoteCandidate, so it will be displayed as host every time. Could you add a case SDP_ICE_CANDIDATE_PARSER_STATE_TYPE: block to parse this? https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/blob/master/src/source/Ice/IceAgent.c#L313

disa6302 commented 3 years ago

Oops. I will spend some time to look into this. No ETA yet.

MushMal commented 3 years ago

Perhaps I am not understanding the feature. Is the ask to parse out the host type from the remote and print it out or keep it and utilize it somehow? If we are talking about printout in verbose mode then I am sure the code somewhere prints out the entire string anyway.

This feature request will need more information before transacted.

yuma-m commented 3 years ago

The purpose is to print the remote candidate type for debugging. With the latest C SDK, the raw remote candidate is printed as BASE64 encoded string.

2021-01-08 01:08:09 DEBUG   lwsWssCallbackRoutine(): Client receive 2021-01-08 01:08:09 DEBUG   lwsWssCallbackRoutine(): Client receive {"messagePayload":"eyJjYW5kaWRhdGUiOiJjYW5kaWRhdGU6MjY5NDU0OTM3NyAxIHVkcCA0MTc1NDg3OSAzLjExMi4xMzAuNzggNTMzMjQgdHlwIHJlbGF5IHJhZGRyIDI3LjAuMy4xNDUgcnBvcnQgMjI1NDYgZ2VuZXJhdGlvbiAwIHVmcmFnIFRaM2YgbmV0d29yay1pZCAzIG5ldHdvcmstY29zdCA1MCIsInNkcE1pZCI6IjAiLCJzZHBNTGluZUluZGV4IjowfQ==","messageType":"ICE_CANDIDATE","senderClientId":"OK29AA5HZC"}
$ echo "eyJjYW5kaWRhdGUiOiJjYW5kaWRhdGU6MjY5NDU0OTM3NyAxIHVkcCA0MTc1NDg3OSAzLjExMi4xMzAuNzggNTMzMjQgdHlwIHJlbGF5IHJhZGRyIDI3LjAuMy4xNDUgcnBvcnQgMjI1NDYgZ2VuZXJhdGlvbiAwIHVmcmFnIFRaM2YgbmV0d29yay1pZCAzIG5ldHdvcmstY29zdCA1MCIsInNkcE1pZCI6IjAiLCJzZHBNTGluZUluZGV4IjowfQ==" | base64 -D
{"candidate":"candidate:2694549377 1 udp 41754879 3.112.130.78 53324 typ relay raddr 27.0.3.145 rport 22546 generation 0 ufrag TZ3f network-id 3 network-cost 50","sdpMid":"0","sdpMLineIndex":0}

However, it is printed as "host" type in the debug logs.

2021-01-08 01:08:09 DEBUG   iceAgentLogNewCandidate(): New remote ice candidate discovered. Id: I0nskhzS9. Ip: 3.112.130.78:53324. Type: host. Protocol: UDP. Priority: 41754879.
2021-01-08 01:08:10 DEBUG   logSelectedIceCandidatesInformation(): Remote Candidate IP Address: 3.112.130.78
2021-01-08 01:08:10 DEBUG   logSelectedIceCandidatesInformation(): Remote Candidate type: host
2021-01-08 01:08:10 DEBUG   logSelectedIceCandidatesInformation(): Remote Candidate port: 53324
2021-01-08 01:08:10 DEBUG   logSelectedIceCandidatesInformation(): Remote Candidate priority: 41754879
2021-01-08 01:08:10 DEBUG   logSelectedIceCandidatesInformation(): Remote Candidate transport protocol: transport=udp

This behavior is confusing and should be fixed.

ycyang1229 commented 3 years ago

Here is one rough commit on my branch(https://github.com/ycyang1229/amazon-kinesis-video-streams-webrtc-sdk-c/commit/d1dd4edc987b8707dd54423a0fa48fc6f7648b9a). Actually, I think it will affect the behavior of ice agent(https://github.com/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/blob/master/src/source/Ice/IceAgent.c#L405), maybe some abnormal behavior. I noticed this several week ago, but I did not clean my current codebase for a while. If you think this should affect ice agent as well, I can clean this commit up and cut one pull request. thanks.

disa6302 commented 3 years ago

@yuma-m ,

Closing this ticket since the change is merged. Feel free to open a new issue if you have questions. Thank you @ycyang1229 for the PR.