Hardcoded header in IVFWriter #484
Conversation
Pull Request Test Coverage Report for Build 2415
💛 - Coveralls |
|
Nice work @Antonito I think this makes the example more complicated that it needs to be (IMO) feel free to push back. Can you try watch the We can then tell users 'refresh the demo page and when the client times out it will exit' also the code will be easier to follow (less LOC and no channels etc..) |
|
Also the WebRTC allows clients to change resolution on the fly (to adjust with available bandwidth) so we don't have anything we can do. |
There was a problem hiding this comment.
Nice work @Antonito
I think this makes the example more complicated that it needs to be (IMO) feel free to push back.
Can you try watch the
webrtc.ICEConnectionStateinstead? I think the code might read cleaner if when we getfailedwe.Close()both files. https://github.com/pions/webrtc/blob/master/examples/save-to-disk/main.go#L100We can then tell users 'refresh the demo page and when the client times out it will exit' also the code will be easier to follow (less LOC and no channels etc..)
Yeah, I feel the example is being a bit over-complicated. I like your implementation better, it's simpler!
examples/save-to-disk/main.go
Outdated
| @@ -64,6 +64,7 @@ func main() { | |||
| // Set a handler for when a new remote track starts, this handler saves buffers to disk as | |||
| // an ivf file, since we could have multiple video tracks we provide a counter. | |||
| // In your application this is where you would handle/process video | |||
| mediaRecorder := []media.Writer{nil, nil} | |||
There was a problem hiding this comment.
Can you put a sync.Mutex around this. Just need to lock since ICE+OnTrack fire in threads
examples/save-to-disk/main.go
Outdated
| @@ -98,6 +101,21 @@ func main() { | |||
| // This will notify you when the peer has connected/disconnected | |||
| peerConnection.OnICEConnectionStateChange(func(connectionState webrtc.ICEConnectionState) { | |||
| fmt.Printf("Connection State has changed %s \n", connectionState.String()) | |||
|
|
|||
| if connectionState == webrtc.ICEConnectionStateConnected { | |||
| fmt.Println("Ctrl+C the client to stop the demo") | |||
There was a problem hiding this comment.
This means .Close() will never get called right? If you want to support Ctrl-C that would be really cool also!
I would put a note in the README the created ivf and opus files will exist, but headers will only have temporary values and may not play properly if they don't end the example properly.
There was a problem hiding this comment.
Oh this needs to be more explicit then. By client, I meant the JS fiddle thing for example. If you "ctrl+c" it (or close the tab), the Go program will detect the connection is closed, will update the headers, close the files and terminate.
I thought about supporting ctrl+c in the Go example, but also wanted to keep the example simple. I can add it if you think it's relevant!
|
Looks much better! This is great, just a few nits/questions but we could merge now if you don't want to support |
- Added timeout to save-to-disk example - Writing number of IVF frames when closing the file - Fixed typos Relates to #471
Relates to #471
The framecount of the IVF header is now updated when we call the
Closemethod.I'm considering passing the
widthandheightfields as parameters toNew()method, and keeping a constan 30 framerate. Any thoughts on this?