VLprojects / webrtc-issue-detector

Diagnostic tool and troubleshooter for WebRTC applications with Mean Opinion Score (MOS) calculator
https://github.com/VLprojects/webrtc-issue-detector/
MIT License
87 stars 16 forks source link

Send RTCpeerconnection as params: WebRTCIssueDetectorConstructorParams #13

Closed nbulankin closed 7 months ago

nbulankin commented 8 months ago

Is it possible add feature for class WebRTCIssueDetector param RTCPeerConnection, to use it without auto-detector. like this:

const webRtcIssueDetector = new WebRTCIssueDetector(RTCPeerConnection, { params });

Chonne commented 8 months ago

After reading the code, I tried this and it worked: webRtcIssueDetector.handleNewPeerConnection(myPeerConnection);

evgmel commented 8 months ago

Hi @nbulankin! Thanks for your question. Of course, you can open a PR with your changes (in case Chonne's answer didn't help). But it's necessary to save backward compatibility.

hellofanengineer commented 8 months ago

@Chonne

After reading the code, I tried this and it worked: webRtcIssueDetector.handleNewPeerConnection(myPeerConnection);

Did you have to do anything to get the detector running? If you call this without also calling watchNewPeerConnections then the internal #running flag is never set to true so handleNewPeerConnection doesn't actually do anything.

I'm also looking for a way to manage the issue detector on a per connection basis because I have many connections and I need to be able to tell which connection is having an issue. As far as I can tell, there's no easy way to that with the existing WebRTCIssueDetector class.

evgmel commented 8 months ago

@hellofanengineer Could you please describe your case why you cannot call watchNewPeerConnections method?

If you call this without also calling watchNewPeerConnections then the internal #running flag is never set to true

Currently when you create an instance of WebRTCIssueDetector class the wrapper of window.RTCPeerConnection is created, so that when a new RTCPeerConnection() is instantiated this methid WebrtcIssueDetector.handleNewPeerConnection(pc) is called automatically (manual call of handleNewPeerConnection() method can be skipped, because the wrapper does the actual call). But in current implementation the reporting cannot be started without calling watchNewPeerConnections() method (could you describe the flow, when you expect it to start stats parsing?)

I'm also looking for a way to manage the issue detector on a per connection basis

Do you mean you want to create an instance of WebRTCIssueDetector for each instance of RTCPeerConnection? Or you want to attach an identifier to an RTCPeerConnection provided to handleNewPeerConnection(pc) method?

hellofanengineer commented 8 months ago

@evgmel I have an app that displays a grid of video cameras. Each cell in the grid is a peer connection and each cell is a UI component so I would like to be able to also manage a connection's webrtc issues within the same component. Ideally when the peer connection is created I could create an issue detector and start it reporting.

I also need to know exactly which component and issue or network score applies to. For example, if I want to show a network bar for each connection, I don't think there's an easy way to know which connection is associated when the network score event is generated.

Do you mean you want to create an instance of WebRTCIssueDetector for each instance of RTCPeerConnection? Or you want to attach an identifier to an RTCPeerConnection provided to handleNewPeerConnection(pc) method?

Ideally yes to the first. I'd create an new WebRTCIssueDetector and then call a start/add function that takes a RTCPeerConnection

evgmel commented 8 months ago

@hellofanengineer Unfortunately, now it's not implemented. The only one WebRTC issue detector instance is created for all the RTCPeerConnections. Because now it wraps the origin connection class. To implement per-connection basis it's necessary to do some changes in the current code (they should be backward compatible). If you have free time you can open a PR with the required changes. Any improvements are welcome and will be checked by our community 👍

hellofanengineer commented 8 months ago

Thanks. I'll try to get to next week.

evgmel commented 7 months ago

Thanks to @hellofanengineer . A new functionality discussed in this issue has been added (this PR)