mirror of
https://github.com/m1k1o/neko.git
synced 2026-11-03 00:11:48 -06:00
fix(capture): derive video ids from pipelines and guard empty list (#711)
when NEKO_CAPTURE_VIDEO_PIPELINES is set without capture.video.ids the VideoIDs list stayed empty, so capture.Video().IDs() returned [] and the first websocket signal request crashed the whole server with "index out of range [0] with length 0" in signalRequest. now we fill VideoIDs from the configured pipeline keys (sorted by id) whenever no ids are set explicitly, and signalRequest returns a plain error instead of panicking when there is no video stream at all. added tests for the config loader, the capture manager and the signal handler so this path stays covered.
This commit is contained in:
committed by
GitHub
parent
bdd0428a60
commit
a2cb38dd10
@@ -0,0 +1,48 @@
|
|||||||
|
package capture
|
||||||
|
|
||||||
|
import (
|
||||||
|
"reflect"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/spf13/cobra"
|
||||||
|
"github.com/spf13/viper"
|
||||||
|
|
||||||
|
"github.com/m1k1o/neko/server/internal/config"
|
||||||
|
)
|
||||||
|
|
||||||
|
// captureConfigFromEnv runs the production config flow (init flags, read env,
|
||||||
|
// populate values) and returns the resulting capture config.
|
||||||
|
func captureConfigFromEnv(t *testing.T, pipelines string) *config.Capture {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
viper.Reset()
|
||||||
|
viper.SetEnvPrefix("NEKO")
|
||||||
|
viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
|
||||||
|
viper.AutomaticEnv()
|
||||||
|
|
||||||
|
t.Setenv("NEKO_CAPTURE_VIDEO_PIPELINES", pipelines)
|
||||||
|
|
||||||
|
cfg := &config.Capture{}
|
||||||
|
cmd := &cobra.Command{}
|
||||||
|
if err := cfg.Init(cmd); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
cfg.Set()
|
||||||
|
return cfg
|
||||||
|
}
|
||||||
|
|
||||||
|
// End to end check for the reported panic: NEKO_CAPTURE_VIDEO_PIPELINES alone
|
||||||
|
// must leave the capture manager with a usable default video stream.
|
||||||
|
func TestVideoIDsAvailableWithOnlyPipelinesEnv(t *testing.T) {
|
||||||
|
const pipeline = "ximagesrc display-name={display} show-pointer=false ! appsink name=appsink"
|
||||||
|
cfg := captureConfigFromEnv(t, `{"high":"`+pipeline+`","medium":"`+pipeline+`","low":"`+pipeline+`"}`)
|
||||||
|
|
||||||
|
manager := New(nil, cfg)
|
||||||
|
|
||||||
|
got := manager.Video().IDs()
|
||||||
|
want := []string{"high", "low", "medium"}
|
||||||
|
if !reflect.DeepEqual(got, want) {
|
||||||
|
t.Fatalf("capture Video().IDs() = %v, want %v (empty list panicked signalRequest)", got, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -3,6 +3,7 @@ package config
|
|||||||
import (
|
import (
|
||||||
"os"
|
"os"
|
||||||
"reflect"
|
"reflect"
|
||||||
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"github.com/go-viper/mapstructure/v2"
|
"github.com/go-viper/mapstructure/v2"
|
||||||
@@ -423,6 +424,20 @@ func (s *Capture) Set() {
|
|||||||
log.Warn().Msg("you are setting both single video pipeline and multiple video pipelines, ignoring single video pipeline")
|
log.Warn().Msg("you are setting both single video pipeline and multiple video pipelines, ignoring single video pipeline")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If no video ids were configured explicitly, derive them from the configured
|
||||||
|
// pipelines so the capture manager always has at least one stream to select.
|
||||||
|
// Without this, NEKO_CAPTURE_VIDEO_PIPELINES alone left VideoIDs empty and the
|
||||||
|
// first WebRTC signal request panicked on videos[0].
|
||||||
|
if len(s.VideoIDs) == 0 && len(s.VideoPipelines) > 0 {
|
||||||
|
ids := make([]string, 0, len(s.VideoPipelines))
|
||||||
|
for id := range s.VideoPipelines {
|
||||||
|
ids = append(ids, id)
|
||||||
|
}
|
||||||
|
sort.Strings(ids)
|
||||||
|
s.VideoIDs = ids
|
||||||
|
log.Warn().Strs("video_ids", ids).Msg("no capture.video.ids configured, using all video pipelines sorted by id")
|
||||||
|
}
|
||||||
|
|
||||||
s.VideoShowPointer = viper.GetBool("capture.video.show_pointer")
|
s.VideoShowPointer = viper.GetBool("capture.video.show_pointer")
|
||||||
if viper.IsSet("capture.video.show_pointer") {
|
if viper.IsSet("capture.video.show_pointer") {
|
||||||
for k, p := range s.VideoPipelines {
|
for k, p := range s.VideoPipelines {
|
||||||
|
|||||||
@@ -0,0 +1,62 @@
|
|||||||
|
package config
|
||||||
|
|
||||||
|
import (
|
||||||
|
"reflect"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/spf13/cobra"
|
||||||
|
"github.com/spf13/viper"
|
||||||
|
)
|
||||||
|
|
||||||
|
// loadCaptureFromEnvWithIDs mirrors loadCaptureFromEnv but also allows setting
|
||||||
|
// capture.video.ids directly, so we can check that explicit ids are preserved.
|
||||||
|
func loadCaptureFromEnvWithIDs(t *testing.T, pipelines string, ids []string) *Capture {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
viper.Reset()
|
||||||
|
viper.SetEnvPrefix("NEKO")
|
||||||
|
viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
|
||||||
|
viper.AutomaticEnv()
|
||||||
|
|
||||||
|
t.Setenv("NEKO_CAPTURE_VIDEO_PIPELINES", pipelines)
|
||||||
|
|
||||||
|
c := &Capture{}
|
||||||
|
cmd := &cobra.Command{}
|
||||||
|
if err := c.Init(cmd); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if ids != nil {
|
||||||
|
viper.Set("capture.video.ids", ids)
|
||||||
|
}
|
||||||
|
|
||||||
|
c.Set()
|
||||||
|
return c
|
||||||
|
}
|
||||||
|
|
||||||
|
// Regression test for the panic in signalRequest: when video pipelines are
|
||||||
|
// configured without an explicit capture.video.ids list, VideoIDs must be
|
||||||
|
// derived from the pipeline keys so the capture manager always has a default
|
||||||
|
// stream to hand out.
|
||||||
|
func TestVideoIDsDerivedFromPipelines(t *testing.T) {
|
||||||
|
const pipeline = "ximagesrc display-name={display} show-pointer=false ! appsink name=appsink"
|
||||||
|
|
||||||
|
t.Run("multiple pipelines without ids populate sorted ids", func(t *testing.T) {
|
||||||
|
c := loadCaptureFromEnvWithIDs(t, `{"high":"`+pipeline+`","medium":"`+pipeline+`","low":"`+pipeline+`"}`, nil)
|
||||||
|
|
||||||
|
want := []string{"high", "low", "medium"}
|
||||||
|
if !reflect.DeepEqual(c.VideoIDs, want) {
|
||||||
|
t.Fatalf("expected video ids %v derived from pipeline keys, got %v (an empty list makes signalRequest panic)", want, c.VideoIDs)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("explicit ids are preserved", func(t *testing.T) {
|
||||||
|
c := loadCaptureFromEnvWithIDs(t, `{"high":"`+pipeline+`","low":"`+pipeline+`"}`, []string{"low", "high"})
|
||||||
|
|
||||||
|
want := []string{"low", "high"}
|
||||||
|
if !reflect.DeepEqual(c.VideoIDs, want) {
|
||||||
|
t.Fatalf("expected explicit video ids %v to be preserved, got %v", want, c.VideoIDs)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
@@ -30,6 +30,9 @@ func (h *MessageHandlerCtx) signalRequest(session types.Session, payload *messag
|
|||||||
// use default first video, if not provided
|
// use default first video, if not provided
|
||||||
if video.Selector == nil {
|
if video.Selector == nil {
|
||||||
videos := h.capture.Video().IDs()
|
videos := h.capture.Video().IDs()
|
||||||
|
if len(videos) == 0 {
|
||||||
|
return errors.New("no video streams available")
|
||||||
|
}
|
||||||
video.Selector = &types.StreamSelector{
|
video.Selector = &types.StreamSelector{
|
||||||
ID: videos[0],
|
ID: videos[0],
|
||||||
Type: types.StreamSelectorTypeExact,
|
Type: types.StreamSelectorTypeExact,
|
||||||
|
|||||||
@@ -0,0 +1,117 @@
|
|||||||
|
package handler
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/pion/webrtc/v4"
|
||||||
|
|
||||||
|
"github.com/m1k1o/neko/server/pkg/types"
|
||||||
|
"github.com/m1k1o/neko/server/pkg/types/message"
|
||||||
|
)
|
||||||
|
|
||||||
|
// The fakes below embed the real interfaces so we only implement the methods
|
||||||
|
// signalRequest actually calls.
|
||||||
|
type fakeSession struct {
|
||||||
|
types.Session
|
||||||
|
profile types.MemberProfile
|
||||||
|
}
|
||||||
|
|
||||||
|
func (s *fakeSession) ID() string { return "test-session" }
|
||||||
|
|
||||||
|
func (s *fakeSession) Profile() types.MemberProfile { return s.profile }
|
||||||
|
|
||||||
|
func (s *fakeSession) PrivateModeEnabled() bool { return false }
|
||||||
|
|
||||||
|
func (s *fakeSession) Send(string, any) {}
|
||||||
|
|
||||||
|
type fakeWebRTC struct {
|
||||||
|
types.WebRTCManager
|
||||||
|
peer *fakePeer
|
||||||
|
}
|
||||||
|
|
||||||
|
func (w *fakeWebRTC) ICEServers() []types.ICEServer { return nil }
|
||||||
|
|
||||||
|
func (w *fakeWebRTC) CreatePeer(types.Session) (*webrtc.SessionDescription, types.WebRTCPeer, error) {
|
||||||
|
w.peer = &fakePeer{}
|
||||||
|
return &webrtc.SessionDescription{SDP: "fake-offer"}, w.peer, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
type fakePeer struct {
|
||||||
|
types.WebRTCPeer
|
||||||
|
video types.PeerVideoRequest
|
||||||
|
}
|
||||||
|
|
||||||
|
func (p *fakePeer) SetPaused(bool) error { return nil }
|
||||||
|
|
||||||
|
func (p *fakePeer) SetVideo(v types.PeerVideoRequest) error {
|
||||||
|
p.video = v
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
func (p *fakePeer) Video() types.PeerVideo { return types.PeerVideo{} }
|
||||||
|
|
||||||
|
func (p *fakePeer) SetAudio(types.PeerAudioRequest) error { return nil }
|
||||||
|
|
||||||
|
func (p *fakePeer) Audio() types.PeerAudio { return types.PeerAudio{} }
|
||||||
|
|
||||||
|
type fakeCapture struct {
|
||||||
|
types.CaptureManager
|
||||||
|
ids []string
|
||||||
|
}
|
||||||
|
|
||||||
|
func (c *fakeCapture) Video() types.EncodedStreamSelector {
|
||||||
|
return &fakeSelector{ids: c.ids}
|
||||||
|
}
|
||||||
|
|
||||||
|
type fakeSelector struct {
|
||||||
|
types.EncodedStreamSelector
|
||||||
|
ids []string
|
||||||
|
}
|
||||||
|
|
||||||
|
func (s *fakeSelector) IDs() []string { return s.ids }
|
||||||
|
|
||||||
|
func newSignalTestHandler(capture types.CaptureManager) (*MessageHandlerCtx, *fakeWebRTC) {
|
||||||
|
webrtcManager := &fakeWebRTC{}
|
||||||
|
h := &MessageHandlerCtx{
|
||||||
|
capture: capture,
|
||||||
|
webrtc: webrtcManager,
|
||||||
|
}
|
||||||
|
return h, webrtcManager
|
||||||
|
}
|
||||||
|
|
||||||
|
func watchSession() *fakeSession {
|
||||||
|
return &fakeSession{profile: types.MemberProfile{CanWatch: true}}
|
||||||
|
}
|
||||||
|
|
||||||
|
// On master, an empty video id list made signalRequest index videos[0] and
|
||||||
|
// panic the whole server. It must return a descriptive error instead.
|
||||||
|
func TestSignalRequestNoVideoStreams(t *testing.T) {
|
||||||
|
h, _ := newSignalTestHandler(&fakeCapture{ids: nil})
|
||||||
|
|
||||||
|
payload := &message.SignalRequest{}
|
||||||
|
if err := h.signalRequest(watchSession(), payload); err == nil {
|
||||||
|
t.Fatal("expected an error when no video streams are configured, got nil")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// When streams exist and the client sends no selector, the first video id is
|
||||||
|
// used as the default stream.
|
||||||
|
func TestSignalRequestDefaultsToFirstVideo(t *testing.T) {
|
||||||
|
h, webrtcManager := newSignalTestHandler(&fakeCapture{ids: []string{"high", "low", "medium"}})
|
||||||
|
|
||||||
|
payload := &message.SignalRequest{}
|
||||||
|
if err := h.signalRequest(watchSession(), payload); err != nil {
|
||||||
|
t.Fatalf("unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
selector := webrtcManager.peer.video.Selector
|
||||||
|
if selector == nil {
|
||||||
|
t.Fatal("expected a default selector to be set")
|
||||||
|
}
|
||||||
|
if selector.ID != "high" {
|
||||||
|
t.Fatalf("expected default video id %q, got %q", "high", selector.ID)
|
||||||
|
}
|
||||||
|
if selector.Type != types.StreamSelectorTypeExact {
|
||||||
|
t.Fatalf("expected exact selector type, got %v", selector.Type)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user