Skip to content

Commit

Permalink
PbReader fix: audio packets from anonymous users results in panic (#37)
Browse files Browse the repository at this point in the history
For each user an Audio Codec is created on the fly with the first
received packet. The Protobuf Message contains a UserID field
which should be set by the client. The UserID is used to ensure
that the audio frames are correcty decoded and that simultaneously
received packets from multiple users are not mixed.
In case a packet with an empty UserID is received, the userID
will be set to 'unknown-client' and a warning is logged.
  • Loading branch information
dh1tw committed Apr 4, 2022
1 parent 5093f0b commit 29928cc
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 54 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ jobs:
OS: darwin
ARCH: arm64
steps:
- name: Set up Go 1.17
- name: Set up Go 1.18
uses: actions/setup-go@v1
id: go
with:
go-version: 1.17
go-version: 1.18
- uses: actions/checkout@v1
with:
submodules: true
Expand Down
3 changes: 2 additions & 1 deletion audio/sinks/pbWriter/pbWriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/dh1tw/gosamplerate"
"github.com/dh1tw/remoteAudio/audio"
"github.com/dh1tw/remoteAudio/audiocodec/opus"
"github.com/dh1tw/remoteAudio/utils"
"github.com/golang/protobuf/proto"

sbAudio "github.com/dh1tw/remoteAudio/sb_audio"
Expand Down Expand Up @@ -45,7 +46,7 @@ func NewPbWriter(opts ...Option) (*PbWriter, error) {
DeviceName: "ProtoBufWriter",
Channels: 1,
FramesPerBuffer: 960,
UserID: "myCallsign",
UserID: fmt.Sprintf("user-%s", utils.RandStringRunes(5)),
},
buffer: make([]byte, 10000),
volume: 0.7,
Expand Down
25 changes: 19 additions & 6 deletions audio/sources/pbReader/pbReader.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ import (
// received from the network.
type PbReader struct {
sync.RWMutex
enabled bool
name string
decoders map[string]audiocodec.Decoder
decoder audiocodec.Decoder
callback audio.OnDataCb
lastUser string
enabled bool
name string
decoders map[string]audiocodec.Decoder
decoder audiocodec.Decoder
callback audio.OnDataCb
lastUser string
emptyUserIDWarning sync.Once
}

// NewPbReader is the constructor for a PbReader object.
Expand Down Expand Up @@ -104,6 +105,14 @@ func (pbr *PbReader) Enqueue(data []byte) error {
return nil
}

if len(msg.GetUserId()) == 0 {
msg.UserId = "unknown-client"
pbr.emptyUserIDWarning.Do(func() {
log.Println("incoming audio frames from unknown user; consider setting the username on the client")
log.Println("simultaneous audio frames from multiple anonymous users will result in distorted audio")
})
}

codecName := msg.GetCodec().String()

switch codecName {
Expand Down Expand Up @@ -141,6 +150,10 @@ func (pbr *PbReader) Enqueue(data []byte) error {
pbr.lastUser = txUser
}

if pbr.decoder == nil {
return fmt.Errorf("no decoder set for audio frames from user: '%s'", txUser)
}

num, err := pbr.decoder.Decode(msg.Data, buf)
if err != nil {
// in case the txUser has switched from stereo to mono
Expand Down
9 changes: 8 additions & 1 deletion cmd/client_nats.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/dh1tw/remoteAudio/audiocodec/opus"
"github.com/dh1tw/remoteAudio/proxy"
"github.com/dh1tw/remoteAudio/trx"
"github.com/dh1tw/remoteAudio/utils"
"github.com/dh1tw/remoteAudio/webserver"
"github.com/gordonklaus/portaudio"
"github.com/nats-io/nats.go"
Expand Down Expand Up @@ -253,11 +254,17 @@ func natsAudioClient(cmd *cobra.Command, args []string) {
exit(err)
}

userName := natsUsername
if len(userName) == 0 {
userName = fmt.Sprintf("client-%s", utils.RandStringRunes(5))
log.Printf("username not set; auto generated unique username '%s'\n", userName)
}

toNetwork, err := pbWriter.NewPbWriter(
pbWriter.Encoder(opusEncoder),
pbWriter.Channels(iChannels),
pbWriter.FramesPerBuffer(audioFramesPerBuffer),
pbWriter.UserID(natsUsername),
pbWriter.UserID(userName),
)
if err != nil {
exit(err)
Expand Down
1 change: 1 addition & 0 deletions cmd/server_nats.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ func natsAudioServer(cmd *cobra.Command, args []string) {
pbWriter.Channels(iChannels),
pbWriter.FramesPerBuffer(audioFramesPerBuffer),
pbWriter.ToWireCb(ns.toWireCb),
pbWriter.UserID(serverName),
)
if err != nil {
exit(err)
Expand Down
16 changes: 10 additions & 6 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,16 @@ require (
github.com/go-audio/audio v1.0.0
github.com/go-audio/wav v1.0.0
github.com/golang/protobuf v1.5.2
github.com/gordonklaus/portaudio v0.0.0-20200911161147-bb74aa485641
github.com/gordonklaus/portaudio v0.0.0-20220320131553-cc649ad523c1
github.com/gorilla/mux v1.8.0
github.com/gorilla/websocket v1.4.2
github.com/gorilla/websocket v1.5.0
github.com/magiconair/properties v1.8.6 // indirect
github.com/nats-io/nats.go v1.13.0
github.com/spf13/cobra v1.2.1
github.com/spf13/viper v1.10.0
google.golang.org/protobuf v1.27.1
gopkg.in/hraban/opus.v2 v2.0.0-20211030232353-1a9beeaf0764
github.com/spf13/afero v1.8.2 // indirect
github.com/spf13/cobra v1.4.0
github.com/spf13/viper v1.10.1
golang.org/x/sys v0.0.0-20220403205710-6acee93ad0eb // indirect
google.golang.org/protobuf v1.28.0
gopkg.in/hraban/opus.v2 v2.0.0-20220302220929-eeacdbcb92d0
gopkg.in/ini.v1 v1.66.4 // indirect
)
Loading

0 comments on commit 29928cc

Please sign in to comment.