Fix doorbird backchannel audio

[oeiber]

• proper session handling
• honor http status codes
• prevent device from being flooded by limiting concurrent audio channels

[felipecrs]

@AlexxIT audio mixing would be nice for all 2-way audio sources. It would improve 2-way audio reliability and allow for more advanced use cases like keeping some ambient music playing in camera, while still being able to speak to camera. Or mixing TTS announcements, Assist, and what not.

What do you think? I can raise a separate issue for it.

[AlexxIT]

Audio mixing will work only for PCM-like formats. Not very universal.
But you can create separate issue.

[oeiber]

Is something still open here because it was not merged with version 1.9.10?

[AlexxIT]

I released the update quickly. There was no time to review this PR.

[AlexxIT]

A lot of work has been done here. But it's very much out of the go2rtc architecture.

• Using a singleton as a client. I understand why this is done. But other go2rtc sources don't work like that. It violates the overall logic.
• Logs inside pkg will create chaos in the overall log output.
• Audio mixing shouldn't be a single client's concern. It should be part of the entire architecture.

This code may work. And it works well. But when go2rtc needs to be improved, this part could become a major problem.

[oeiber]

A lot of work has been done here. But it's very much out of the go2rtc architecture.

• Using a singleton as a client. I understand why this is done. But other go2rtc sources don't work like that. It violates the overall logic.
• Logs inside pkg will create chaos in the overall log output.
• Audio mixing shouldn't be a single client's concern. It should be part of the entire architecture.

This code may work. And it works well. But when go2rtc needs to be improved, this part could become a major problem.

Do you have any constructive suggestions on how to tackle the problems to meet the 'overall logic'?
Otherwise, I'll delete the merge request and give up on the whole thing...

[AlexxIT]

Does the device work with the current release code? If only one WebRTC client is open at a time.

[oeiber]

No, it doesn't. Device is getting flooded with audio packets.

[AlexxIT]

Does this happen even with a single two-way client? I see your code primarily fix the issue of multiple clients.

[oeiber]

Yes, it happens also with a single client.
In your original version, each audio packet sent to doorbird starts a new tcp connection. After a while the device gets unreachable.
To fix the problem, I included a connection handler, which ensures, data sent to the device uses the same tcp connection. There's also a periodic connection check, which starts a new session, after the first one gets broken for any reason:

func (c *Client) Start() error {
	if c.conn == nil {
		return nil
	}

	go func() {
		ticker := time.NewTicker(SenderCleanupInterval)
		defer ticker.Stop()
		for range ticker.C {
			c.cleanupOrphanedSenders()
		}
	}()

	// Start a heartbeat goroutine to periodically check connection health
	go func() {
		heartbeat := time.NewTicker(HeartbeatInterval)
		defer heartbeat.Stop()

		for range heartbeat.C {
			c.mu.RLock()
			conn := c.conn
			c.mu.RUnlock()

			if conn != nil {
				// Try to write a small amount of silence to keep connection alive
				silence := make([]byte, 160) // 20ms of silence at 8kHz
				_ = conn.SetWriteDeadline(time.Now().Add(core.ConnDeadline))
				if _, err := conn.Write(silence); err != nil {
					fmt.Printf("DoorBird: Heartbeat write failed: %v\n", err)
					// Don't break here, let the main read loop handle it
				}
			}
		}
	}()

	// The main loop now just monitors for any unexpected data or connection errors
	// DoorBird typically doesn't send data back, so we use a very long timeout
	buf := make([]byte, 1)
	connectionStart := time.Now()
	for {
		_ = c.conn.SetReadDeadline(time.Now().Add(ConnectionReadTimeout))
		n, err := c.conn.Read(buf)
		if err != nil {
			elapsed := time.Since(connectionStart)
			fmt.Printf("DoorBird: Connection failed after %v, error: %v\n", elapsed, err)
			c.cleanup()
			cltMu.Lock()
			delete(cltMap, c.URL)
			cltMu.Unlock()
			return err
		}
		if n > 0 {
			fmt.Printf("DoorBird: Unexpected data received: %v\n", buf[:n])
		}
	}
}

[AlexxIT]

This code may not work:

go2rtc/pkg/doorbird/backchannel.go

Lines 90 to 93 in 2b5f942

	func (c *Client) Start() (err error) {
	_, err = c.conn.Read(nil)
	return
	}

It is assumed that Start holds the connection. For the producer, it's simple. The Start receives the data.

In the case of backchannel, Start only needs to maintain the connection. Maybe send pings, as you do.

Maybe it needs to repeat this code:

go2rtc/pkg/isapi/client.go

Lines 130 to 132 in 2b5f942

	// just block until c.conn closed
	b := make([]byte, 1)
	_, _ = c.conn.Read(b)

[oeiber]

I can give it a try. But there's still the question how to handle multiple baxkchannel sessions.

[AlexxIT]

If there is only one line in the config, there will be only one session. Go2rtc simply won't let you run yet another. It's built into the core.

But if you open multiple browsers, they may compete for the right to send their data to the same session. This issue hasn't been debugged in go2rtc.

[oeiber]

This code may not work:

go2rtc/pkg/doorbird/backchannel.go

Lines 90 to 93 in 2b5f942

	func (c *Client) Start() (err error) {
	_, err = c.conn.Read(nil)
	return
	}

It is assumed that Start holds the connection. For the producer, it's simple. The Start receives the data.

In the case of backchannel, Start only needs to maintain the connection. Maybe send pings, as you do.

Maybe it needs to repeat this code:

go2rtc/pkg/isapi/client.go

Lines 130 to 132 in 2b5f942

	// just block until c.conn closed
	b := make([]byte, 1)
	_, _ = c.conn.Read(b)

Hi @AlexxIT, seems it is working with the few lines of code adopted into the Client function!

[AlexxIT]

It would be great if you could debug your camera and make another PR. It's hard for me to do this because I don't have this camera.

Your code from this PR may be useful when it becomes possible to make a centralized mixer for multiple clients of the same backchannel.

[oeiber]

Here we go :-) #1895