revid: variable bitrate default for HTTP mode, also wrote some testing for config validation

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 == "" {

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 <saxon@ausocean.org>
+
+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)
+		}
+	}
+}

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: