From 1e3b4b1ab89c82adfdffc00d624ef9956f76ff38 Mon Sep 17 00:00:00 2001 From: Dan Kortschak Date: Mon, 10 Dec 2018 09:39:20 +1030 Subject: [PATCH] cmd/revid-cli,revid: reduce stringly typing in config/flags --- cmd/revid-cli/main.go | 148 ++++++++++++++----------------------- revid/config.go | 166 +++++++++++++++--------------------------- revid/revid.go | 41 ++++------- revid/senders.go | 4 +- 4 files changed, 129 insertions(+), 230 deletions(-) diff --git a/cmd/revid-cli/main.go b/cmd/revid-cli/main.go index 32b18be6..3fda0862 100644 --- a/cmd/revid-cli/main.go +++ b/cmd/revid-cli/main.go @@ -33,6 +33,7 @@ import ( "os" "runtime/pprof" "strconv" + "strings" "time" "bitbucket.org/ausocean/av/revid" @@ -101,22 +102,21 @@ func handleFlags() revid.Config { output2Ptr = flag.String("Output2", "", "The second output type: Http, Rtmp, File, Udp, Rtp") rtmpMethodPtr = flag.String("RtmpMethod", "", "The method used to send over rtmp: Ffmpeg, Librtmp") packetizationPtr = flag.String("Packetization", "", "The method of data packetisation: Flv, Mpegts, None") - quantizationModePtr = flag.String("QuantizationMode", "", "Whether quantization if on or off (variable bitrate): On, Off") + quantizePtr = flag.Bool("Quantize", false, "Quantize input (non-variable bitrate)") verbosityPtr = flag.String("Verbosity", "", "Verbosity: Info, Warning, Error, Fatal") - framesPerClipPtr = flag.String("FramesPerClip", "", "Number of frames per clip sent") + framesPerClipPtr = flag.Uint("FramesPerClip", 0, "Number of frames per clip sent") rtmpUrlPtr = flag.String("RtmpUrl", "", "Url of rtmp endpoint") - bitratePtr = flag.String("Bitrate", "", "Bitrate of recorded video") + bitratePtr = flag.Uint("Bitrate", 0, "Bitrate of recorded video") outputFileNamePtr = flag.String("OutputFileName", "", "The directory of the output file") inputFileNamePtr = flag.String("InputFileName", "", "The directory of the input file") - heightPtr = flag.String("Height", "", "Height in pixels") - widthPtr = flag.String("Width", "", "Width in pixels") - frameRatePtr = flag.String("FrameRate", "", "Frame rate of captured video") + heightPtr = flag.Uint("Height", 0, "Height in pixels") + widthPtr = flag.Uint("Width", 0, "Width in pixels") + frameRatePtr = flag.Uint("FrameRate", 0, "Frame rate of captured video") httpAddressPtr = flag.String("HttpAddress", "", "Destination address of http posts") - quantizationPtr = flag.String("Quantization", "", "Desired quantization value: 0-40") - timeoutPtr = flag.String("Timeout", "", "Http timeout in seconds") - intraRefreshPeriodPtr = flag.String("IntraRefreshPeriod", "", "The IntraRefreshPeriod i.e. how many keyframes we send") - verticalFlipPtr = flag.String("VerticalFlip", "", "Flip video vertically: Yes, No") - horizontalFlipPtr = flag.String("HorizontalFlip", "", "Flip video horizontally: Yes, No") + quantizationPtr = flag.Uint("Quantization", 0, "Desired quantization value: 0-40") + intraRefreshPeriodPtr = flag.Uint("IntraRefreshPeriod", 0, "The IntraRefreshPeriod i.e. how many keyframes we send") + verticalFlipPtr = flag.Bool("VerticalFlip", false, "Flip video vertically: Yes, No") + horizontalFlipPtr = flag.Bool("HorizontalFlip", false, "Flip video horizontally: Yes, No") logPathPtr = flag.String("LogPath", defaultLogPath, "Path for logging files (default is /var/log/netsender/)") rtpAddrPtr = flag.String("RtpAddr", "", "Rtp destination address: : (port is generally 6970-6999)") ) @@ -218,16 +218,6 @@ func handleFlags() revid.Config { logger.Log(smartlogger.Error, pkg+"bad packetization argument") } - switch *quantizationModePtr { - case "QuantizationOn": - cfg.QuantizationMode = revid.QuantizationOn - case "QuantizationOff": - cfg.QuantizationMode = revid.QuantizationOff - case "": - default: - logger.Log(smartlogger.Error, pkg+"bad quantization mode argument") - } - switch *verbosityPtr { case "No": cfg.LogLevel = smartlogger.Fatal @@ -239,32 +229,10 @@ func handleFlags() revid.Config { logger.Log(smartlogger.Error, pkg+"bad verbosity argument") } - switch *horizontalFlipPtr { - case "No": - cfg.FlipHorizontal = false - case "Yes": - cfg.FlipHorizontal = true - case "": - cfg.FlipHorizontal = false - default: - logger.Log(smartlogger.Error, pkg+"bad horizontal flip option") - } - - switch *verticalFlipPtr { - case "No": - cfg.FlipVertical = false - case "Yes": - cfg.FlipVertical = true - case "": - cfg.FlipVertical = false - default: - logger.Log(smartlogger.Error, pkg+"bad vertical flip option") - } - - fpc, err := strconv.Atoi(*framesPerClipPtr) - if err == nil && fpc > 0 { - cfg.FramesPerClip = fpc - } + cfg.Quantize = *quantizePtr + cfg.FlipHorizontal = *horizontalFlipPtr + cfg.FlipVertical = *verticalFlipPtr + cfg.FramesPerClip = *framesPerClipPtr cfg.RtmpUrl = *rtmpUrlPtr cfg.Bitrate = *bitratePtr cfg.OutputFileName = *outputFileNamePtr @@ -274,7 +242,6 @@ func handleFlags() revid.Config { cfg.FrameRate = *frameRatePtr cfg.HttpAddress = *httpAddressPtr cfg.Quantization = *quantizationPtr - cfg.Timeout = *timeoutPtr cfg.IntraRefreshPeriod = *intraRefreshPeriodPtr cfg.RtpAddress = *rtpAddrPtr @@ -406,79 +373,76 @@ func updateRevid(ns *netsender.Sender, rv *revid.Revid, cfg revid.Config, vars m continue } case "FramesPerClip": - fpc, err := strconv.Atoi(value) - if fpc > 0 && err == nil { - cfg.FramesPerClip = fpc - } else { - logger.Log(smartlogger.Warning, pkg+"invalid FramesPerClip param", "value", value) + f, err := strconv.ParseUint(value, 10, 0) + if err != nil { + logger.Log(smartlogger.Warning, pkg+"invalid framesperclip param", "value", value) + break } + cfg.FramesPerClip = uint(f) case "RtmpUrl": cfg.RtmpUrl = value case "Bitrate": - asInt, err := strconv.Atoi(value) - if asInt > 0 && err == nil { - cfg.Bitrate = value - } else { - logger.Log(smartlogger.Warning, pkg+"invalid Bitrate param", "value", value) + r, err := strconv.ParseUint(value, 10, 0) + if err != nil { + logger.Log(smartlogger.Warning, pkg+"invalid framerate param", "value", value) + break } + cfg.Bitrate = uint(r) case "OutputFileName": cfg.OutputFileName = value case "InputFileName": cfg.InputFileName = value case "Height": - asInt, err := strconv.Atoi(value) - if asInt > 0 && err == nil { - cfg.Height = value - } else { - logger.Log(smartlogger.Warning, pkg+"invalid Height param", "value", value) + h, err := strconv.ParseUint(value, 10, 0) + if err != nil { + logger.Log(smartlogger.Warning, pkg+"invalid height param", "value", value) + break } + cfg.Height = uint(h) case "Width": - asInt, err := strconv.Atoi(value) - if asInt > 0 && err == nil { - cfg.Width = value - } else { - logger.Log(smartlogger.Warning, pkg+"invalid Width param", "value", value) + w, err := strconv.ParseUint(value, 10, 0) + if err != nil { + logger.Log(smartlogger.Warning, pkg+"invalid width param", "value", value) + break } + cfg.Width = uint(w) case "FrameRate": - asInt, err := strconv.Atoi(value) - if asInt > 0 && err == nil { - cfg.FrameRate = value - } else { - logger.Log(smartlogger.Warning, pkg+"invalid FrameRate param", "value", value) + r, err := strconv.ParseUint(value, 10, 0) + if err != nil { + logger.Log(smartlogger.Warning, pkg+"invalid framerate param", "value", value) + break } + cfg.FrameRate = uint(r) case "HttpAddress": cfg.HttpAddress = value case "Quantization": - asInt, err := strconv.Atoi(value) - if asInt > 0 && err == nil { - cfg.Quantization = value - } else { - logger.Log(smartlogger.Warning, pkg+"invalid Quantization param", "value", value) - } - case "Timeout": - asInt, err := strconv.Atoi(value) - if asInt > 0 && err == nil { - cfg.Timeout = value + q, err := strconv.ParseUint(value, 10, 0) + if err != nil { + logger.Log(smartlogger.Warning, pkg+"invalid quantization param", "value", value) + break } + cfg.Quantization = uint(q) case "IntraRefreshPeriod": - asInt, err := strconv.Atoi(value) - if asInt > 0 && err == nil { - cfg.IntraRefreshPeriod = value + p, err := strconv.ParseUint(value, 10, 0) + if err != nil { + logger.Log(smartlogger.Warning, pkg+"invalid intrarefreshperiod param", "value", value) + break } + cfg.IntraRefreshPeriod = uint(p) case "HorizontalFlip": - switch value { - case "Yes": + switch strings.ToLower(value) { + case "true": cfg.FlipHorizontal = true - case "No": + case "false": cfg.FlipHorizontal = false default: logger.Log(smartlogger.Warning, pkg+"invalid HorizontalFlip param", "value", value) } case "VerticalFlip": - switch value { - case "Yes": + switch strings.ToLower(value) { + case "true": cfg.FlipVertical = true - case "No": + case "false": cfg.FlipVertical = false default: logger.Log(smartlogger.Warning, pkg+"invalid VerticalFlip param", "value", value) diff --git a/revid/config.go b/revid/config.go index 06a4b8da..8becc91b 100644 --- a/revid/config.go +++ b/revid/config.go @@ -29,7 +29,6 @@ package revid import ( "errors" - "strconv" "bitbucket.org/ausocean/utils/smartlogger" ) @@ -37,32 +36,36 @@ import ( // Config provides parameters relevant to a revid instance. A new config must // be passed to the constructor. type Config struct { - Input uint8 - InputCodec uint8 - Output1 uint8 - Output2 uint8 - RtmpMethod uint8 - Packetization uint8 - QuantizationMode uint8 - LogLevel int8 + LogLevel int8 + + Input uint8 + InputCodec uint8 + Output1 uint8 + Output2 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 int + FramesPerClip uint RtmpUrl string - Bitrate string + Bitrate uint OutputFileName string InputFileName string - Height string - Width string - FrameRate string + Height uint + Width uint + FrameRate uint HttpAddress string - Quantization string - Timeout string - IntraRefreshPeriod string + Quantization uint + IntraRefreshPeriod uint RtpAddress string Logger Logger SendRetry bool @@ -98,20 +101,18 @@ const ( defaultInput = Raspivid defaultOutput = Http defaultPacketization = Flv - defaultFrameRate = "25" - defaultWidth = "1280" - defaultHeight = "720" - defaultIntraRefreshPeriod = "100" - defaultTimeout = "0" - defaultQuantization = "40" - defaultBitrate = "400000" + defaultFrameRate = 25 + defaultWidth = 1280 + defaultHeight = 720 + defaultIntraRefreshPeriod = 100 + defaultTimeout = 0 + defaultQuantization = 40 + defaultBitrate = 400000 defaultQuantizationMode = QuantizationOff defaultFramesPerClip = 1 - defaultVerticalFlip = No - defaultHorizontalFlip = No httpFramesPerClip = 560 defaultInputCodec = H264 - defaultVerbosity = No + defaultVerbosity = No // FIXME(kortschak): This makes no sense whatsoever. No is currently 15. defaultRtpAddr = "localhost:6970" ) @@ -129,17 +130,6 @@ func (c *Config) Validate(r *Revid) error { return errors.New("bad LogLevel defined in config") } - switch c.QuantizationMode { - case QuantizationOn: - case QuantizationOff: - case NothingDefined: - c.Logger.Log(smartlogger.Warning, pkg+"no quantization mode defined, defaulting", - "quantizationMode", QuantizationOff) - c.QuantizationMode = QuantizationOff - default: - return errors.New("bad QuantizationMode defined in config") - } - switch c.Input { case Raspivid: case File: @@ -153,29 +143,23 @@ func (c *Config) Validate(r *Revid) error { switch c.InputCodec { case H264: - if c.Bitrate != "" && c.Quantization != "" { - bitrate, err := strconv.Atoi(c.Bitrate) - if err != nil { - return errors.New("bitrate not an integer") - } - quantization, err := strconv.Atoi(c.Quantization) - if err != nil { - return errors.New("quantization not an integer") - } - if (bitrate > 0 && quantization > 0) || (bitrate == 0 && quantization == 0) { - return errors.New("bad bitrate and quantization combination for H264 input") - } + // 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 + } else { + c.Bitrate = defaultBitrate } + + if (c.Bitrate > 0 && c.Quantization > 0) || (c.Bitrate == 0 && c.Quantization == 0) { + return errors.New("bad bitrate and quantization combination for H264 input") + } + case Mjpeg: - if c.Quantization != "" { - quantization, err := strconv.Atoi(c.Quantization) - if err != nil { - return errors.New("quantization not an integer") - } - if quantization > 0 || c.Bitrate == "" { - return errors.New("bad bitrate or quantization for mjpeg input") - } + if c.Quantization > 0 || c.Bitrate == 0 { + return errors.New("bad bitrate or quantization for mjpeg input") } + case NothingDefined: c.Logger.Log(smartlogger.Warning, pkg+"no input codec defined, defaulting", "inputCodec", defaultInputCodec) @@ -183,6 +167,7 @@ func (c *Config) Validate(r *Revid) error { c.Logger.Log(smartlogger.Warning, pkg+"defaulting quantization", "quantization", defaultQuantization) c.Quantization = defaultQuantization + default: return errors.New("bad input codec defined in config") } @@ -234,73 +219,36 @@ func (c *Config) Validate(r *Revid) error { c.FramesPerClip = defaultFramesPerClip } - if c.Width == "" { - c.Logger.Log(smartlogger.Warning, pkg+"no width defined, defaulting", "width", - defaultWidth) + if c.Width == 0 { + c.Logger.Log(smartlogger.Warning, pkg+"no width defined, defaulting", "width", defaultWidth) c.Width = defaultWidth - } else { - if integer, err := strconv.Atoi(c.Width); integer < 0 || err != nil { - return errors.New("width not unsigned integer") - } } - if c.Height == "" { - c.Logger.Log(smartlogger.Warning, pkg+"no height defined, defaulting", "height", - defaultHeight) + if c.Height == 0 { + c.Logger.Log(smartlogger.Warning, pkg+"no height defined, defaulting", "height", defaultHeight) c.Height = defaultHeight - } else { - if integer, err := strconv.Atoi(c.Height); integer < 0 || err != nil { - return errors.New("height not unsigned integer") - } } - if c.FrameRate == "" { - c.Logger.Log(smartlogger.Warning, pkg+"no frame rate defined, defaulting", "fps", - defaultFrameRate) + if c.FrameRate == 0 { + c.Logger.Log(smartlogger.Warning, pkg+"no frame rate defined, defaulting", "fps", defaultFrameRate) c.FrameRate = defaultFrameRate - } else { - if integer, err := strconv.Atoi(c.FrameRate); integer < 0 || err != nil { - return errors.New("frame rate not unsigned integer") - } } - if c.Bitrate == "" { - c.Logger.Log(smartlogger.Warning, pkg+"no bitrate defined, defaulting", "bitrate", - defaultBitrate) + if c.Bitrate == 0 { + c.Logger.Log(smartlogger.Warning, pkg+"no bitrate defined, defaulting", "bitrate", defaultBitrate) c.Bitrate = defaultBitrate - } else { - if integer, err := strconv.Atoi(c.Bitrate); integer < 0 || err != nil { - return errors.New("bitrate not unsigned integer") - } } - if c.Timeout == "" { - c.Logger.Log(smartlogger.Warning, pkg+"no timeout defined, defaulting", "timeout", defaultTimeout) - c.Timeout = defaultTimeout - } else { - if integer, err := strconv.Atoi(c.Timeout); integer < 0 || err != nil { - return errors.New("timeout not unsigned integer") - } - } - - if c.IntraRefreshPeriod == "" { - c.Logger.Log(smartlogger.Warning, pkg+"no intra refresh defined, defaulting", "intraRefresh", - defaultIntraRefreshPeriod) + if c.IntraRefreshPeriod == 0 { + c.Logger.Log(smartlogger.Warning, pkg+"no intra refresh defined, defaulting", "intraRefresh", defaultIntraRefreshPeriod) c.IntraRefreshPeriod = defaultIntraRefreshPeriod - } else { - if integer, err := strconv.Atoi(c.IntraRefreshPeriod); integer < 0 || err != nil { - return errors.New("intra refresh not unsigned integer") - } } - if c.Quantization == "" { - c.Logger.Log(smartlogger.Warning, pkg+"no quantization defined, defaulting", "quantization", - defaultQuantization) + if c.Quantization == 0 { + c.Logger.Log(smartlogger.Warning, pkg+"no quantization defined, defaulting", "quantization", defaultQuantization) c.Quantization = defaultQuantization - } else { - if integer, err := strconv.Atoi(c.Quantization); integer < 0 || integer > 51 || err != nil { - return errors.New("quantisation not unsigned integer or is over threshold") - } + } else if c.Quantization > 51 { + return errors.New("quantisation is over threshold") } if c.RtpAddress == "" { diff --git a/revid/revid.go b/revid/revid.go index e883bb45..4a5d1be7 100644 --- a/revid/revid.go +++ b/revid/revid.go @@ -133,7 +133,7 @@ var prevTime = now type packer struct { owner *Revid - packetCount int + packetCount uint } // Write implements the io.Writer interface. @@ -227,7 +227,7 @@ func (r *Revid) reset(config Config) error { } r.destination[outNo] = s case FfmpegRtmp: - s, err := newFfmpegSender(config.RtmpUrl, r.config.FrameRate) + s, err := newFfmpegSender(config.RtmpUrl, fmt.Sprint(r.config.FrameRate)) if err != nil { return err } @@ -247,10 +247,7 @@ func (r *Revid) reset(config Config) error { } r.destination[outNo] = s case Rtp: - // TODO: framerate in config should probably be an int, make conversions early - // when setting config fields in revid-cli - fps, _ := strconv.Atoi(r.config.FrameRate) - s, err := newRtpSender(r.config.RtpAddress, r.config.Logger.Log, fps) + s, err := newRtpSender(r.config.RtpAddress, r.config.Logger.Log, r.config.FrameRate) if err != nil { return err } @@ -292,12 +289,10 @@ func (r *Revid) reset(config Config) error { r.encoder = stream.NopEncoder(&r.packer) case Mpegts: r.config.Logger.Log(smartlogger.Info, pkg+"using MPEGTS packetisation") - frameRate, _ := strconv.ParseFloat(r.config.FrameRate, 64) - r.encoder = mts.NewEncoder(&r.packer, frameRate) + r.encoder = mts.NewEncoder(&r.packer, float64(r.config.FrameRate)) case Flv: r.config.Logger.Log(smartlogger.Info, pkg+"using FLV packetisation") - frameRate, _ := strconv.Atoi(r.config.FrameRate) - r.encoder, err = flv.NewEncoder(&r.packer, true, true, frameRate) + r.encoder, err = flv.NewEncoder(&r.packer, true, true, int(r.config.FrameRate)) if err != nil { return err } @@ -447,10 +442,10 @@ func (r *Revid) startRaspivid() error { "--output", "-", "--nopreview", "--timeout", disabled, - "--width", r.config.Width, - "--height", r.config.Height, - "--bitrate", r.config.Bitrate, - "--framerate", r.config.FrameRate, + "--width", fmt.Sprint(r.config.Width), + "--height", fmt.Sprint(r.config.Height), + "--bitrate", fmt.Sprint(r.config.Bitrate), + "--framerate", fmt.Sprint(r.config.FrameRate), } if r.config.FlipHorizontal { args = append(args, "--hflip") @@ -465,10 +460,10 @@ func (r *Revid) startRaspivid() error { args = append(args, "--codec", "H264", "--inline", - "--intra", r.config.IntraRefreshPeriod, + "--intra", fmt.Sprint(r.config.IntraRefreshPeriod), ) - if r.config.QuantizationMode == QuantizationOn { - args = append(args, "-qp", r.config.Quantization) + if r.config.Quantize { + args = append(args, "-qp", fmt.Sprint(r.config.Quantization)) } case Mjpeg: args = append(args, "--codec", "MJPEG") @@ -476,11 +471,6 @@ func (r *Revid) startRaspivid() error { r.config.Logger.Log(smartlogger.Info, pkg+"raspivid args", "raspividArgs", strings.Join(args, " ")) r.cmd = exec.Command("raspivid", args...) - d, err := strconv.Atoi(r.config.FrameRate) - if err != nil { - return err - } - delay := time.Second / time.Duration(d) stdout, err := r.cmd.StdoutPipe() if err != nil { return err @@ -491,6 +481,7 @@ func (r *Revid) startRaspivid() error { } r.config.Logger.Log(smartlogger.Info, pkg+"reading camera data") + delay := time.Second / time.Duration(r.config.FrameRate) err = r.lexTo(r.encoder, stdout, delay) r.config.Logger.Log(smartlogger.Info, pkg+"finished reading camera data") return err @@ -498,11 +489,7 @@ func (r *Revid) startRaspivid() error { // setupInputForFile sets things up for getting input from a file func (r *Revid) setupInputForFile() error { - fps, err := strconv.Atoi(r.config.FrameRate) - if err != nil { - return err - } - delay := time.Second / time.Duration(fps) + delay := time.Second / time.Duration(r.config.FrameRate) f, err := os.Open(r.config.InputFileName) if err != nil { diff --git a/revid/senders.go b/revid/senders.go index 9309e940..6606350e 100644 --- a/revid/senders.go +++ b/revid/senders.go @@ -371,14 +371,14 @@ type rtpSender struct { encoder *rtp.Encoder } -func newRtpSender(addr string, log func(lvl int8, msg string, args ...interface{}), fps int) (*rtpSender, error) { +func newRtpSender(addr string, log func(lvl int8, msg string, args ...interface{}), fps uint) (*rtpSender, error) { conn, err := net.Dial("udp", addr) if err != nil { return nil, err } s := &rtpSender{ log: log, - encoder: rtp.NewEncoder(conn, fps), + encoder: rtp.NewEncoder(conn, int(fps)), } return s, nil }