selkies-project / selkies-gstreamer

Open-Source Low-Latency Accelerated Linux WebRTC HTML5 Remote Desktop Streaming Platform for Self-Hosting, Containers, Kubernetes, or Cloud/HPC
https://selkies-project.github.io/selkies-gstreamer/
Mozilla Public License 2.0
381 stars 50 forks source link

Audio randomly gets disabled after latest changes to webrtcbin #109

Closed ehfd closed 7 months ago

ehfd commented 1 year ago

Because the option to turn on and off audio was removed, it is expected that audio should always be on.

However, the audio goes off randomly.

It could be because we should check that both audio and video WebRTC streams, as well as the RTCDataChannel, are all connected before we declare the connection is established successfully. There is a possibility that this was omitted in the latest commits.

This has been reproduced by myself and a user who has reported the issue to me.

The metrics menu may, but not necessarily show the following:

Audio Stats
Latency: 0 ms
Codec: NA
Bit rate: 0.00 kbps
ehfd commented 1 year ago

This issue has been tagged as Urgent.

@danisla

ehfd commented 1 year ago

I believe that this issue is because the audio and video WebRTC elements do not cross-check that both protocols have been connected successfully.

danisla commented 1 year ago

How do you reproduce this?

ehfd commented 1 year ago

@danisla Try to reload the web interface multiple times. Especially from a non-local connection passing through the internet. It's not guaranteed to happen every time, but I have at least one other user reproducing this.

The core problem is that the current code seemingly does not ensure that both WebRTC streams are established before exposing the web interface to the user.

ehfd commented 1 year ago
Audio Stats
Latency: 0 ms
Codec: NA
Bit rate: 0.00 kbps

It seems like this is shown even when audio is active. Stats are not shown.

ehfd commented 1 year ago

@Xosrov Could I ask how you solved this problem?

Xosrov commented 1 year ago

Hey, I don't recall such an issue, but I did have a look at the new source code and I notice something:

if audio_only:
    self.build_audio_pipeline()
else:
    self.build_video_pipeline()

This implies the start_pipeline function is called twice; once for video and once for audio. Am I getting this right? If so, the self.pipeline object is being modified and that could cause the signaling process to be disturbed. There needs to be more logging to know for sure, though.

The way I had implemented it was to first preroll the capturing + encoding + packetizing pipeline, then attach and sync a specific webrtcbin to the already running pipeline, which In my experience made initializing the stream a lot faster.

Some other things (unrelated to this topic,) that you could consider adding to POSSIBLY improve the whole stream experience:

I hope this helps.

ehfd commented 1 year ago

Thanks for the massive help. @Xosrov While I'm busy dealing with other things, I'll make sure to bake your suggestions into the code.

ehfd commented 11 months ago
  • Since there are two webrtcbins now, I would enable NACK on both. I'm not sure why you're doing it here; as the if-statement implies you only use it for one and not both?
  • I also used to enable FEC specifically in audio streams, but again I haven't tested to see if they have an effect on audio quality.

Applied these two things in https://github.com/selkies-project/selkies-gstreamer/commit/6d16d0052a4ec80e136996d7634c68a2cb350a90. Let's see if fixing #131 is the core thing, though.

ehfd commented 11 months ago

I understand that this is the same issue as #131. Please reopen if there are additional issues.

ehfd commented 10 months ago

There is still an issue where one should ensure both WebRTC (video, audio) streams are connected before the start button is shown, and the reload button should be shown after failure in either of the two streams.

ehfd commented 7 months ago

Please reopen if there are additional issues.