From ce8295bb3613ab22fb5a2b93a7443c43047a20a6 Mon Sep 17 00:00:00 2001 From: Saxon Date: Sat, 24 Aug 2019 14:02:24 +0930 Subject: [PATCH 1/3] revid: variable bitrate default for HTTP mode, also wrote some testing for config validation --- revid/config.go | 71 ++++++++++++++++---------------- revid/config_test.go | 97 ++++++++++++++++++++++++++++++++++++++++++++ revid/revid.go | 10 ++--- 3 files changed, 138 insertions(+), 40 deletions(-) create mode 100644 revid/config_test.go diff --git a/revid/config.go b/revid/config.go index 82e06115..701c0753 100644 --- a/revid/config.go +++ b/revid/config.go @@ -109,7 +109,7 @@ const ( defaultWidth = 1280 defaultHeight = 720 defaultIntraRefreshPeriod = 100 - defaultQuantization = 40 + defaultQuantization = 30 defaultBitrate = 400000 // Audio defaults. @@ -170,10 +170,6 @@ type Config struct { // localhost:6970. MPEGT-TS packetization is used. Outputs []uint8 - // Quantize specifies whether the input to revid will have constant or variable - // bitrate, if configurable with the chosen input. Raspivid supports quantization. - Quantize bool - // RTMPURL specifies the Rtmp output destination URL. This must be defined if // RTMP is to be used as an output. RTMPURL string @@ -252,9 +248,14 @@ type Config struct { MTSRBElementSize int // The element size in bytes of the MTS sender RingBuffer. } +// Validation errors. +var ( + errInvalidQuantization = errors.New("invalid quantization") +) + // Validate checks for any errors in the config fields and defaults settings // if particular parameters have not been defined. -func (c *Config) Validate(r *Revid) error { +func (c *Config) Validate() error { switch c.LogLevel { case logger.Debug: case logger.Info: @@ -277,12 +278,6 @@ func (c *Config) Validate(r *Revid) error { switch c.InputCodec { case codecutil.H264: - // FIXME(kortschak): This is not really what we want. - // Configuration really needs to be rethought here. - if c.Quantize && c.Quantization == 0 { - c.Quantization = defaultQuantization - } - case codecutil.MJPEG: if c.Quantization > 0 || c.Bitrate == 0 { return errors.New("bad bitrate or quantization for mjpeg input") @@ -304,22 +299,32 @@ func (c *Config) Validate(r *Revid) error { if c.Outputs == nil { c.Logger.Log(logger.Info, pkg+"no output defined, defaulting", "output", defaultOutput) c.Outputs = append(c.Outputs, defaultOutput) - } else { - for i, o := range c.Outputs { - switch o { - case File: - case RTMP: - if c.RTMPURL == "" { - c.Logger.Log(logger.Info, pkg+"no RTMP URL: falling back to HTTP") - c.Outputs[i] = HTTP - // FIXME(kortschak): Does this want the same line as below? - // c.FramesPerClip = httpFramesPerClip - break - } - case HTTP, RTP: - default: - return errors.New("bad output type defined in config") + } + + var haveRTMPOut bool + for i, o := range c.Outputs { + switch o { + case File: + case RTMP: + haveRTMPOut = true + if c.Bitrate == 0 { + c.Bitrate = defaultBitrate } + c.Quantization = 0 + if c.RTMPURL == "" { + c.Logger.Log(logger.Info, pkg+"no RTMP URL: falling back to HTTP") + c.Outputs[i] = HTTP + break + } + case HTTP, RTP: + if !haveRTMPOut { + c.Bitrate = 0 + if c.Quantization == 0 { + c.Quantization = defaultQuantization + } + } + default: + return errors.New("bad output type defined in config") } } @@ -373,9 +378,8 @@ func (c *Config) Validate(r *Revid) error { c.WriteRate = defaultWriteRate } - if c.Bitrate == 0 { - c.Logger.Log(logger.Info, pkg+"no bitrate defined, defaulting", "bitrate", defaultBitrate) - c.Bitrate = defaultBitrate + if c.Bitrate < 0 { + return errors.New("invalid bitrate") } if c.IntraRefreshPeriod == 0 { @@ -383,11 +387,8 @@ func (c *Config) Validate(r *Revid) error { c.IntraRefreshPeriod = defaultIntraRefreshPeriod } - if c.Quantization == 0 { - c.Logger.Log(logger.Info, pkg+"no quantization defined, defaulting", "quantization", defaultQuantization) - c.Quantization = defaultQuantization - } else if c.Quantization > 51 { - return errors.New("quantisation is over threshold") + if c.Quantization != 0 && (c.Quantization < 10 || c.Quantization > 40) { + return errInvalidQuantization } if c.RTPAddress == "" { diff --git a/revid/config_test.go b/revid/config_test.go new file mode 100644 index 00000000..53f81ea9 --- /dev/null +++ b/revid/config_test.go @@ -0,0 +1,97 @@ +/* +DESCRIPTION + config_test.go provides testing for configuration functionality found in config.go. + +AUTHORS + Saxon A. Nelson-Milton + +LICENSE + Copyright (C) 2019 the Australian Ocean Lab (AusOcean) + + It is free software: you can redistribute it and/or modify them + under the terms of the GNU General Public License as published by the + Free Software Foundation, either version 3 of the License, or (at your + option) any later version. + + It is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + for more details. + + You should have received a copy of the GNU General Public License + in gpl.txt. If not, see http://www.gnu.org/licenses. +*/ + +package revid + +import ( + "fmt" + "testing" +) + +type dumbLogger struct{} + +func (dl *dumbLogger) SetLevel(l int8) {} +func (dl *dumbLogger) Log(l int8, m string, p ...interface{}) {} + +func TestValidate(t *testing.T) { + tests := []struct { + in Config + check func(c Config) error + err error + }{ + { + in: Config{Outputs: []uint8{HTTP}, Logger: &dumbLogger{}}, + check: func(c Config) error { + if c.Bitrate != 0 { + return fmt.Errorf("did not get expected bitrate. Got: %v, Want: %v", c.Bitrate, 0) + } + if c.Quantization != defaultQuantization { + return fmt.Errorf("did not get expected quantization. Got: %v, Want: %v", c.Quantization, defaultQuantization) + } + return nil + }, + }, + { + in: Config{Outputs: []uint8{RTMP}, Logger: &dumbLogger{}}, + check: func(c Config) error { + if c.Bitrate != defaultBitrate { + return fmt.Errorf("did not get expected bitrate. Got: %v, Want: %v", c.Bitrate, defaultBitrate) + } + if c.Quantization != 0 { + return fmt.Errorf("did not get expected quantization. Got: %v, Want: %v", c.Quantization, 0) + } + return nil + }, + }, + { + in: Config{Outputs: []uint8{HTTP}, Quantization: 50, Logger: &dumbLogger{}}, + check: func(c Config) error { return nil }, + err: errInvalidQuantization, + }, + { + in: Config{Outputs: []uint8{HTTP}, Quantization: 20, Logger: &dumbLogger{}}, + check: func(c Config) error { + if c.Bitrate != 0 { + return fmt.Errorf("did not get expected bitrate. Got: %v, Want: %v", c.Bitrate, 0) + } + if c.Quantization != 20 { + return fmt.Errorf("did not get expected quantization. Got: %v, Want: %v", c.Quantization, 20) + } + return nil + }, + }, + } + + for i, test := range tests { + err := (&test.in).Validate() + if err != test.err { + t.Errorf("did not get expected error for test %d\nGot: %v\nWant: %v\n", i, err, test.err) + } + + err = test.check(test.in) + if err != nil { + t.Errorf("test %d failed with err: %v", i, err) + } + } +} diff --git a/revid/revid.go b/revid/revid.go index f18a6a43..9e11844a 100644 --- a/revid/revid.go +++ b/revid/revid.go @@ -197,7 +197,7 @@ func (r *Revid) reset(config Config) error { // revid config. func (r *Revid) setConfig(config Config) error { r.config.Logger = config.Logger - err := config.Validate(r) + err := config.Validate() if err != nil { return errors.New("Config struct is bad: " + err.Error()) } @@ -450,12 +450,12 @@ func (r *Revid) Update(vars map[string]string) error { case "HttpAddress": r.config.HTTPAddress = value case "Quantization": - q, err := strconv.ParseUint(value, 10, 0) + v, err := strconv.Atoi(value) if err != nil { - r.config.Logger.Log(logger.Warning, pkg+"invalid quantization param", "value", value) + r.config.Logger.Log(logger.Warning, pkg+"invalid quantization param", "value", v) break } - r.config.Quantization = uint(q) + r.config.Quantization = uint(v) case "IntraRefreshPeriod": p, err := strconv.ParseUint(value, 10, 0) if err != nil { @@ -579,7 +579,7 @@ func (r *Revid) startRaspivid() (func() error, error) { "--inline", "--intra", fmt.Sprint(r.config.IntraRefreshPeriod), ) - if r.config.Quantize { + if r.config.Quantization != 0 { args = append(args, "-qp", fmt.Sprint(r.config.Quantization)) } case codecutil.MJPEG: From 9eb155dfed8c8469355b11e5bd7300feb044d163 Mon Sep 17 00:00:00 2001 From: Saxon Date: Sat, 24 Aug 2019 14:05:34 +0930 Subject: [PATCH 2/3] revid-cli: removed use of config quantize param in revid-cli --- cmd/revid-cli/main.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cmd/revid-cli/main.go b/cmd/revid-cli/main.go index c98513cc..53f38833 100644 --- a/cmd/revid-cli/main.go +++ b/cmd/revid-cli/main.go @@ -111,7 +111,6 @@ func handleFlags() revid.Config { inputCodecPtr = flag.String("InputCodec", "H264", "The codec of the input: H264, Mjpeg, PCM, ADPCM") inputPtr = flag.String("Input", "", "The input type: Raspivid, File, v4l, Audio, RTSP") rtspURLPtr = flag.String("RTSPURL", "", "The URL for an RTSP server.") - quantizePtr = flag.Bool("Quantize", false, "Quantize input (non-variable bitrate)") verbosityPtr = flag.String("Verbosity", "Info", "Verbosity: Debug, Info, Warning, Error, Fatal") rtpAddrPtr = flag.String("RtpAddr", "", "Rtp destination address: : (port is generally 6970-6999)") logPathPtr = flag.String("LogPath", defaultLogPath, "The log path") @@ -235,7 +234,6 @@ func handleFlags() revid.Config { } cfg.RTSPURL = *rtspURLPtr - cfg.Quantize = *quantizePtr cfg.Rotation = *rotationPtr cfg.FlipHorizontal = *horizontalFlipPtr cfg.FlipVertical = *verticalFlipPtr From d3909182098a3ff8057ba546df53ff4a68d13f1a Mon Sep 17 00:00:00 2001 From: Saxon Date: Sat, 24 Aug 2019 14:53:49 +0930 Subject: [PATCH 3/3] revid: corrected logic for RTMPURL fallback and fixed bug in test --- revid/config.go | 3 ++- revid/config_test.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/revid/config.go b/revid/config.go index 701c0753..42ab2b5d 100644 --- a/revid/config.go +++ b/revid/config.go @@ -314,8 +314,9 @@ func (c *Config) Validate() error { if c.RTMPURL == "" { c.Logger.Log(logger.Info, pkg+"no RTMP URL: falling back to HTTP") c.Outputs[i] = HTTP - break + haveRTMPOut = false } + fallthrough case HTTP, RTP: if !haveRTMPOut { c.Bitrate = 0 diff --git a/revid/config_test.go b/revid/config_test.go index 53f81ea9..eb248c93 100644 --- a/revid/config_test.go +++ b/revid/config_test.go @@ -53,7 +53,7 @@ func TestValidate(t *testing.T) { }, }, { - in: Config{Outputs: []uint8{RTMP}, Logger: &dumbLogger{}}, + in: Config{Outputs: []uint8{RTMP}, RTMPURL: "dummURL", Logger: &dumbLogger{}}, check: func(c Config) error { if c.Bitrate != defaultBitrate { return fmt.Errorf("did not get expected bitrate. Got: %v, Want: %v", c.Bitrate, defaultBitrate)