add shutdown to close

[craymond12]

Description

Reference issue

Fixes #...

[Sean-Der]

@craymond12 Do we not do a graceful close currently without this?

[craymond12]

@craymond12 Do we not do a graceful close currently without this?

sorry, i was supposed to push on my fork to test things Currently when we close the webrtc connection we do not send the shutdown message

This often cause the webrtc stream to hang until timeout on the producer side when the consumer leave Chrome do send it

[Sean-Der]

@craymond12 we can run the tests here! If you open the PR again I can run the tests and if they pass we can merge things.

Would it be possible to write a test? Otherwise someone might undo your fix accidentally.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 65.11%. Comparing base (64a837f) to head (61abac2). Report is 7 commits behind head on master.

:exclamation: There is a different number of reports uploaded between BASE (64a837f) and HEAD (61abac2). Click for more details.

HEAD has 2 uploads less than BASE

Flag	BASE (64a837f)	HEAD (61abac2)
go	2	0

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #2885       +/-   ##
===========================================
- Coverage   78.96%   65.11%   -13.85%     
===========================================
  Files          89       67       -22     
  Lines        8461     3262     -5199     
===========================================
- Hits         6681     2124     -4557     
+ Misses       1294     1011      -283     
+ Partials      486      127      -359     

Flag	Coverage Δ
go	?
wasm	65.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[craymond12]

@craymond12 we can run the tests here! If you open the PR again I can run the tests and if they pass we can merge things.

Would it be possible to write a test? Otherwise someone might undo your fix accidentally.

this was kind of hackish, would have to think more about the context sent to the shutdown

[craymond12]

@craymond12 we can run the tests here! If you open the PR again I can run the tests and if they pass we can merge things.

Would it be possible to write a test? Otherwise someone might undo your fix accidentally.

the root of the issue is a lot of onvif camera support 2 to 4 streams, Having the webrtc hang in the camera for around 30 seconds can be an issue

I intended to explore this behavior in a fork to avoid creating some noise here But once satisfied I will publish a PR here

[craymond12]

@craymond12 we can run the tests here! If you open the PR again I can run the tests and if they pass we can merge things.

Would it be possible to write a test? Otherwise someone might undo your fix accidentally.

I managed to do some test with some real Axis device and it work

I am thinking of having a new peerConnection Close method to which we will be able to pass down the context for cancellation I will be off for 1 week and in my return i will update the pr and look at the tests

[edaniels]

@craymond12 we can run the tests here! If you open the PR again I can run the tests and if they pass we can merge things. Would it be possible to write a test? Otherwise someone might undo your fix accidentally.

I managed to do some test with some real Axis device and it work

I am thinking of having a new peerConnection Close method to which we will be able to pass down the context for cancellation I will be off for 1 week and in my return i will update the pr and look at the tests

Given how new the method is, you can probably put the context into GracefulClose