Merged in comment-config (pull request #199)

revid/config.go: better commenting on Config fields.

Approved-by: Alan Noble <anoble@gmail.com>

cmd/revid-cli/main.go

@@ -108,7 +108,6 @@ func handleFlags() revid.Config {
 		inputPtr              = flag.String("Input", "", "The input type: Raspivid, File, Webcam, RTSP")
 		rtspURLPtr            = flag.String("RTSPURL", "", "The URL for an RTSP server.")
 		inputCodecPtr         = flag.String("InputCodec", "", "The codec of the input: H264, Mjpeg")
-		rtmpMethodPtr         = flag.String("RtmpMethod", "", "The method used to send over rtmp: Ffmpeg, Librtmp")
 		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: <IP>:<port> (port is generally 6970-6999)")
@@ -118,7 +117,6 @@ func handleFlags() revid.Config {
 		outputPathPtr         = flag.String("OutputPath", "", "The directory of the output file")
 		inputFilePtr          = flag.String("InputPath", "", "The directory of the input file")
 		httpAddressPtr        = flag.String("HttpAddress", "", "Destination address of http posts")
-		sendRetryPtr          = flag.Bool("retry", false, "Specify whether a failed send should be retried.")
 		verticalFlipPtr       = flag.Bool("VerticalFlip", false, "Flip video vertically: Yes, No")
 		horizontalFlipPtr     = flag.Bool("HorizontalFlip", false, "Flip video horizontally: Yes, No")
 		framesPerClipPtr      = flag.Uint("FramesPerClip", 0, "Number of frames per clip sent")
@@ -201,27 +199,17 @@ func handleFlags() revid.Config {
 		case "File":
 			cfg.Outputs = append(cfg.Outputs, revid.File)
 		case "Http":
-			cfg.Outputs = append(cfg.Outputs, revid.Http)
+			cfg.Outputs = append(cfg.Outputs, revid.HTTP)
 		case "Rtmp":
-			cfg.Outputs = append(cfg.Outputs, revid.Rtmp)
+			cfg.Outputs = append(cfg.Outputs, revid.RTMP)
 		case "Rtp":
-			cfg.Outputs = append(cfg.Outputs, revid.Rtp)
+			cfg.Outputs = append(cfg.Outputs, revid.RTP)
 		case "":
 		default:
 			log.Log(logger.Error, pkg+"bad output argument", "arg", o)
 		}
 	}
 
-	switch *rtmpMethodPtr {
-	case "Ffmpeg":
-		cfg.RtmpMethod = revid.Ffmpeg
-	case "LibRtmp":
-		cfg.RtmpMethod = revid.LibRtmp
-	case "":
-	default:
-		log.Log(logger.Error, pkg+"bad rtmp method argument")
-	}
-
 	if *configFilePtr != "" {
 		netsender.ConfigFile = *configFilePtr
 	}
@@ -232,18 +220,17 @@ func handleFlags() revid.Config {
 	cfg.FlipHorizontal = *horizontalFlipPtr
 	cfg.FlipVertical = *verticalFlipPtr
 	cfg.FramesPerClip = *framesPerClipPtr
-	cfg.RtmpUrl = *rtmpUrlPtr
+	cfg.RTMPURL = *rtmpUrlPtr
 	cfg.Bitrate = *bitratePtr
 	cfg.OutputPath = *outputPathPtr
 	cfg.InputPath = *inputFilePtr
 	cfg.Height = *heightPtr
 	cfg.Width = *widthPtr
 	cfg.FrameRate = *frameRatePtr
-	cfg.HttpAddress = *httpAddressPtr
+	cfg.HTTPAddress = *httpAddressPtr
 	cfg.Quantization = *quantizationPtr
 	cfg.IntraRefreshPeriod = *intraRefreshPeriodPtr
-	cfg.RtpAddress = *rtpAddrPtr
+	cfg.RTPAddress = *rtpAddrPtr
-	cfg.SendRetry = *sendRetryPtr
 	cfg.Brightness = *brightnessPtr
 	cfg.Saturation = *saturationPtr
 	cfg.Exposure = *exposurePtr

revid/config.go

@@ -33,50 +33,6 @@ import (
 	"bitbucket.org/ausocean/utils/logger"
 )
 
-// Config provides parameters relevant to a revid instance. A new config must
-// be passed to the constructor.
-type Config struct {
-	LogLevel int8
-
-	Input         uint8
-	InputCodec    uint8
-	Outputs       []uint8
-	RtmpMethod    uint8
-	Packetization uint8
-
-	// Quantize specifies whether the input to
-	// revid will have constant or variable
-	// bitrate.
-	Quantize bool
-
-	// FlipHorizonatla and FlipVertical specify
-	// whether video frames should be flipped.
-	FlipHorizontal bool
-	FlipVertical   bool
-
-	FramesPerClip      uint
-	RtmpUrl            string
-	Bitrate            uint
-	OutputPath         string
-	InputPath          string
-	Height             uint
-	Width              uint
-	FrameRate          uint
-	HttpAddress        string
-	Quantization       uint
-	IntraRefreshPeriod uint
-	RtpAddress         string
-	Logger             Logger
-	SendRetry          bool
-	BurstPeriod        uint
-	Rotation           uint
-	Brightness         uint
-	Saturation         int
-	Exposure           string
-	AutoWhiteBalance   string
-	RTSPURL            string
-}
-
 // Possible modes for raspivid --exposure parameter.
 var ExposureModes = [...]string{
 	"auto",
@@ -107,38 +63,35 @@ var AutoWhiteBalanceModes = [...]string{
 	"horizon",
 }
 
-// Enums for config struct
+// Enums to define inputs, outputs and codecs.
 const (
+	// Indicates no option has been set.
 	NothingDefined = iota
+
+	// Input/Output.
+	File
+
+	// Inputs.
 	Raspivid
 	V4L
-	H264Codec
-	File
-	Http
-	H264
-	Mjpeg
-	None
-	Mpegts
-	Ffmpeg
-	Flv
-	LibRtmp
-	QuantizationOn
-	QuantizationOff
-	Yes
-	No
-	Rtmp
-	FfmpegRtmp
-	Udp
-	MpegtsRtp
-	Rtp
 	RTSP
+
+	// Outputs.
+	RTMP
+	RTP
+	HTTP
+	MPEGTS
+
+	// Codecs.
+	H264
+	H265
+	MJPEG
 )
 
 // Default config settings
 const (
 	defaultInput              = Raspivid
-	defaultOutput             = Http
+	defaultOutput             = HTTP
-	defaultPacketization      = Flv
 	defaultFrameRate          = 25
 	defaultWidth              = 1280
 	defaultHeight             = 720
@@ -146,7 +99,6 @@ const (
 	defaultTimeout            = 0
 	defaultQuantization       = 40
 	defaultBitrate            = 400000
-	defaultQuantizationMode   = QuantizationOff
 	defaultFramesPerClip      = 1
 	httpFramesPerClip         = 560
 	defaultInputCodec         = H264
@@ -159,6 +111,120 @@ const (
 	defaultAutoWhiteBalance   = "auto"
 )
 
+// Config provides parameters relevant to a revid instance. A new config must
+// be passed to the constructor. Default values for these fields are defined
+// as consts above.
+type Config struct {
+	// LogLevel is the revid logging verbosity level.
+	// Valid values are defined by enums from the logger package: logger.Debug,
+	// logger.Info, logger.Warning logger.Error, logger.Fatal.
+	LogLevel int8
+
+	// Input defines the input data source.
+	//
+	// Valid values are defined by enums:
+	// Raspivid:
+	//		Read data from a Raspberry Pi Camera.
+	// V4l:
+	//		Read from webcam.
+	// File:
+	// 		Location must be specified in InputPath field.
+	// RTSP:
+	//		RTSPURL must also be defined.
+	Input uint8
+
+	// InputCodec defines the input codec we wish to use, and therefore defines the
+	// lexer for use in the pipeline. This defaults to H264, but H265 is also a
+	// valid option if we expect this from the input.
+	InputCodec uint8
+
+	// Outputs define the outputs we wish to output data too.
+	//
+	// Valid outputs are defined by enums:
+	// File:
+	// 		Location must be defined by the OutputPath field. MPEG-TS packetization
+	//		is used.
+	// HTTP:
+	// 		Destination is defined by the sh field located in /etc/netsender.conf.
+	// 		MPEGT-TS packetization is used.
+	// RTMP:
+	// 		Destination URL must be defined in the RtmpUrl field. FLV packetization
+	//		is used.
+	// RTP:
+	// 		Destination is defined by RtpAddr field, otherwise it will default to
+	//		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
+
+	// FramesPerClip defines the number of packetization units to pack into a clip
+	// per HTTP send.
+	FramesPerClip uint
+
+	// RTMPURL specifies the Rtmp output destination URL. This must be defined if
+	// RTMP is to be used as an output.
+	RTMPURL string
+
+	// RTSPURL specifies the RTSP server URL for RTSP input. This must be defined
+	// when Input is RTSP.
+	RTSPURL string
+
+	// OutputPath defines the output destination for File output. This must be
+	// defined if File output is to be used.
+	OutputPath string
+
+	// InputPath defines the input file location for File Input. This must be
+	// defined if File input is to be used.
+	InputPath string
+
+	// FrameRate defines the input frame rate if configurable by the chosen input.
+	// Raspivid input supports custom framerate.
+	FrameRate uint
+
+	// HTTPAddress defines a custom HTTP destination if we do not wish to use that
+	// defined in /etc/netsender.conf.
+	HTTPAddress string
+
+	// Quantization defines the quantization level, which may be a value between
+	// 0-40. This will only take effect if the Quantize field is true and if we
+	// are using Raspivid input.
+	Quantization uint
+
+	// IntraRefreshPeriod defines the frequency of video parameter NAL units for
+	// Raspivid input.
+	IntraRefreshPeriod uint
+
+	// Logger holds an implementation of the Logger interface as defined in revid.go.
+	// This must be set for revid to work correctly.
+	Logger Logger
+
+	// Brightness and saturation define the brightness and saturation levels for
+	// Raspivid input.
+	Brightness uint
+	Saturation int
+
+	// Exposure defines the exposure mode used by the Raspivid input. Valid modes
+	// are defined in the exported []string ExposureModes defined at the start
+	// of the file.
+	Exposure string
+
+	// AutoWhiteBalance defines the auto white balance mode used by Raspivid input.
+	// Valid modes are defined in the exported []string AutoWhiteBalanceModes
+	// defined at the start of the file.
+	AutoWhiteBalance string
+
+	RTPAddress     string // RTPAddress defines the RTP output destination.
+	BurstPeriod    uint   // BurstPeriod defines the revid burst period in seconds.
+	Rotation       uint   // Rotation defines the video rotation angle in degrees Raspivid input.
+	Height         uint   // Height defines the input video height Raspivid input.
+	Width          uint   // Width  defines the input video width Raspivid input.
+	Bitrate        uint   // Bitrate specifies the input bitrate for Raspivid input.
+	FlipHorizontal bool   // FlipHorizontal flips video horizontally for Raspivid input.
+	FlipVertical   bool   // FlipVertial flips video vertically for Raspivid input.
+}
+
 // 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 {
@@ -194,7 +260,7 @@ func (c *Config) Validate(r *Revid) error {
 			return errors.New("bad bitrate and quantization combination for H264 input")
 		}
 
-	case Mjpeg:
+	case MJPEG:
 		if c.Quantization > 0 || c.Bitrate == 0 {
 			return errors.New("bad bitrate or quantization for mjpeg input")
 		}
@@ -211,28 +277,23 @@ 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)
-		c.Packetization = defaultPacketization
 	} else {
 		for i, o := range c.Outputs {
 			switch o {
 			case File:
-			case Udp:
+			case RTMP:
-			case Rtmp, FfmpegRtmp:
+				if c.RTMPURL == "" {
-				if c.RtmpUrl == "" {
 					c.Logger.Log(logger.Info, pkg+"no RTMP URL: falling back to HTTP")
-					c.Outputs[i] = Http
+					c.Outputs[i] = HTTP
 					// FIXME(kortschak): Does this want the same line as below?
 					//  c.FramesPerClip = httpFramesPerClip
 					break
 				}
 				c.Logger.Log(logger.Info, pkg+"defaulting frames per clip for rtmp out", "framesPerClip", defaultFramesPerClip)
 				c.FramesPerClip = defaultFramesPerClip
-				c.Packetization = Flv
+			case HTTP, RTP:
-				c.SendRetry = true
-			case Http, Rtp:
 				c.Logger.Log(logger.Info, pkg+"defaulting frames per clip for http out", "framesPerClip", httpFramesPerClip)
 				c.FramesPerClip = httpFramesPerClip
-				c.Packetization = Mpegts
 			default:
 				return errors.New("bad output type defined in config")
 			}
@@ -286,8 +347,8 @@ func (c *Config) Validate(r *Revid) error {
 		return errors.New("quantisation is over threshold")
 	}
 
-	if c.RtpAddress == "" {
+	if c.RTPAddress == "" {
-		c.RtpAddress = defaultRtpAddr
+		c.RTPAddress = defaultRtpAddr
 	}
 
 	switch {

revid/revid.go

@@ -230,11 +230,11 @@ func (r *Revid) setupPipeline(mtsEnc, flvEnc func(dst io.WriteCloser, rate int)
 	var w io.WriteCloser
 	for _, out := range r.config.Outputs {
 		switch out {
-		case Http:
+		case HTTP:
 			w = newMtsSender(newHttpSender(r.ns, r.config.Logger.Log), r.config.Logger.Log, rbSize, rbElementSize, 0)
 			mtsSenders = append(mtsSenders, w)
-		case Rtp:
+		case RTP:
-			w, err := newRtpSender(r.config.RtpAddress, r.config.Logger.Log, r.config.FrameRate)
+			w, err := newRtpSender(r.config.RTPAddress, r.config.Logger.Log, r.config.FrameRate)
 			if err != nil {
 				r.config.Logger.Log(logger.Warning, pkg+"rtp connect error", "error", err.Error())
 			}
@@ -245,8 +245,8 @@ func (r *Revid) setupPipeline(mtsEnc, flvEnc func(dst io.WriteCloser, rate int)
 				return err
 			}
 			mtsSenders = append(mtsSenders, w)
-		case Rtmp:
+		case RTMP:
-			w, err := newRtmpSender(r.config.RtmpUrl, rtmpConnectionTimeout, rtmpConnectionMaxTries, r.config.Logger.Log)
+			w, err := newRtmpSender(r.config.RTMPURL, rtmpConnectionTimeout, rtmpConnectionMaxTries, r.config.Logger.Log)
 			if err != nil {
 				r.config.Logger.Log(logger.Warning, pkg+"rtmp connect error", "error", err.Error())
 			}
@@ -382,11 +382,11 @@ func (r *Revid) Update(vars map[string]string) error {
 				case "File":
 					r.config.Outputs[i] = File
 				case "Http":
-					r.config.Outputs[i] = Http
+					r.config.Outputs[i] = HTTP
 				case "Rtmp":
-					r.config.Outputs[i] = Rtmp
+					r.config.Outputs[i] = RTMP
 				case "Rtp":
-					r.config.Outputs[i] = Rtp
+					r.config.Outputs[i] = RTP
 				default:
 					r.config.Logger.Log(logger.Warning, pkg+"invalid output param", "value", value)
 					continue
@@ -394,9 +394,9 @@ func (r *Revid) Update(vars map[string]string) error {
 			}
 
 		case "RtmpUrl":
-			r.config.RtmpUrl = value
+			r.config.RTMPURL = value
 		case "RtpAddress":
-			r.config.RtpAddress = value
+			r.config.RTPAddress = value
 		case "Bitrate":
 			v, err := strconv.ParseUint(value, 10, 0)
 			if err != nil {
@@ -437,7 +437,7 @@ func (r *Revid) Update(vars map[string]string) error {
 			}
 			r.config.Rotation = uint(v)
 		case "HttpAddress":
-			r.config.HttpAddress = value
+			r.config.HTTPAddress = value
 		case "Quantization":
 			q, err := strconv.ParseUint(value, 10, 0)
 			if err != nil {
@@ -543,7 +543,7 @@ func (r *Revid) startRaspivid() (func() error, error) {
 		if r.config.Quantize {
 			args = append(args, "-qp", fmt.Sprint(r.config.Quantization))
 		}
-	case Mjpeg:
+	case MJPEG:
 		args = append(args, "--codec", "MJPEG")
 	}
 	r.config.Logger.Log(logger.Info, pkg+"raspivid args", "raspividArgs", strings.Join(args, " "))

revid/revid_test.go

@@ -148,7 +148,7 @@ func TestResetEncoderSenderSetup(t *testing.T) {
 		encoders []encoder
 	}{
 		{
-			outputs: []uint8{Http},
+			outputs: []uint8{HTTP},
 			encoders: []encoder{
 				{
 					encoderType:  mtsEncoderStr,
@@ -157,7 +157,7 @@ func TestResetEncoderSenderSetup(t *testing.T) {
 			},
 		},
 		{
-			outputs: []uint8{Rtmp},
+			outputs: []uint8{RTMP},
 			encoders: []encoder{
 				{
 					encoderType:  flvEncoderStr,
@@ -166,7 +166,7 @@ func TestResetEncoderSenderSetup(t *testing.T) {
 			},
 		},
 		{
-			outputs: []uint8{Rtp},
+			outputs: []uint8{RTP},
 			encoders: []encoder{
 				{
 					encoderType:  mtsEncoderStr,
@@ -175,7 +175,7 @@ func TestResetEncoderSenderSetup(t *testing.T) {
 			},
 		},
 		{
-			outputs: []uint8{Http, Rtmp},
+			outputs: []uint8{HTTP, RTMP},
 			encoders: []encoder{
 				{
 					encoderType:  mtsEncoderStr,
@@ -188,7 +188,7 @@ func TestResetEncoderSenderSetup(t *testing.T) {
 			},
 		},
 		{
-			outputs: []uint8{Http, Rtp, Rtmp},
+			outputs: []uint8{HTTP, RTP, RTMP},
 			encoders: []encoder{
 				{
 					encoderType:  mtsEncoderStr,
@@ -201,7 +201,7 @@ func TestResetEncoderSenderSetup(t *testing.T) {
 			},
 		},
 		{
-			outputs: []uint8{Rtp, Rtmp},
+			outputs: []uint8{RTP, RTMP},
 			encoders: []encoder{
 				{
 					encoderType:  mtsEncoderStr,
@@ -224,7 +224,7 @@ func TestResetEncoderSenderSetup(t *testing.T) {
 	for testNum, test := range tests {
 		// Create a new config and reset revid with it.
 		const dummyURL = "rtmp://dummy"
-		c := Config{Logger: &testLogger{}, Outputs: test.outputs, RtmpUrl: dummyURL}
+		c := Config{Logger: &testLogger{}, Outputs: test.outputs, RTMPURL: dummyURL}
 		err := rv.setConfig(c)
 		if err != nil {
 			t.Fatalf("unexpected error: %v for test %v", err, testNum)