Skip to content

Commit

Permalink
Enable additional linters and fix issues
Browse files Browse the repository at this point in the history
Enabling following linters in golangci config:

- ireturn
- prealloc
- reassign

As part of linter issues some interfaces such as Commander and Measurer
were removed since their use is limited.

Also some minor spellings are fixed.
  • Loading branch information
jurisevo committed Mar 25, 2024
1 parent fa551d0 commit b8e53a1
Show file tree
Hide file tree
Showing 12 changed files with 38 additions and 62 deletions.
14 changes: 12 additions & 2 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ linters-settings:
min-len: 2
min-occurrences: 3
govet:
check-shadowing: true
shadow:
strict: true
nolintlint:
require-explanation: true
require-specific: true
Expand All @@ -21,7 +22,13 @@ linters-settings:
Copyright ©{{CPY_YEAR}} {{COMPANY}}. All rights reserved.
Use of this source code is governed by a MIT-style
license that can be found in the LICENSE file.
ireturn:
allow:
- anon
- error
- empty
- stdlib
- generic

linters:
disable-all: true
Expand All @@ -44,3 +51,6 @@ linters:
- goheader
- gosec
- exportloopref
- ireturn
- prealloc
- reassign
7 changes: 2 additions & 5 deletions bitrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ import (
"gonum.org/v1/plot/vg/vgimg"
)

// Make sure BitrateApp implements Commander interface.
var _ Commander = (*BitrateApp)(nil)

// BitrateApp is bitrate subcommand context that implements Commander interface.
type BitrateApp struct {
// Configuration object
Expand All @@ -38,8 +35,8 @@ type BitrateApp struct {
gf globalFlags
}

// CreateBitrateCommand will create Commander instance from BitrateApp.
func CreateBitrateCommand() Commander {
// CreateBitrateCommand will instance of BitrateApp.
func CreateBitrateCommand() *BitrateApp {
longHelp := `Subcommand "bitrate" will create bitrate plot for given video file.`
app := &BitrateApp{
fs: flag.NewFlagSet("bitrate", flag.ContinueOnError),
Expand Down
5 changes: 0 additions & 5 deletions common.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ import (
"github.com/evolution-gaming/ease/internal/logging"
)

// Commander interface should be implemented by commands and sub-commands.
type Commander interface {
Run([]string) error
}

// AppError a custom error returned from CLI application.
//
// AppError is handy error type envisioned to be used in CLI's main.
Expand Down
5 changes: 1 addition & 4 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (o ConfigVal[T]) MarshalJSON() ([]byte, error) {
return json.Marshal(o.Value())
}

func CreateDumpConfCommand() Commander {
func CreateDumpConfCommand() *DumpConfApp {
longHelp := `Command "dump-conf" will print actual application configuration taking into account
configuration file provided and default configuration values.
Expand All @@ -261,9 +261,6 @@ Examples:

// Also define command "dump-conf" here.

// Make sure App implements Commander interface.
var _ Commander = (*DumpConfApp)(nil)

// DumpConfApp is subcommand application context that implements Commander interface.
// Although this is very simple application, but for consistency sake is is implemented in
// similar style as other subcommands.
Expand Down
4 changes: 1 addition & 3 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,7 @@ func Test_DumpConfApp_Run(t *testing.T) {
cmd := CreateDumpConfCommand()

// Redirect output to buffer
if c, ok := cmd.(*DumpConfApp); ok {
c.out = commandOutput
}
cmd.out = commandOutput

err := cmd.Run([]string{"-conf", confFile})
assert.NoError(t, err, "Unexpected error running encode")
Expand Down
4 changes: 1 addition & 3 deletions ease_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,7 @@ func Test_RunApp_Run_FlagErrors(t *testing.T) {
cmd := CreateRunCommand()
// Discard usage output so that during test execution test output is
// not flooded with command Usage/Help stuff.
if c, ok := cmd.(*App); ok {
c.fs.SetOutput(io.Discard)
}
cmd.fs.SetOutput(io.Discard)
gotErr := cmd.Run(tc.givenArgs)
assert.ErrorContains(t, gotErr, tc.want)
})
Expand Down
6 changes: 3 additions & 3 deletions internal/analysis/plot.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func CreateCDFPlot(values []float64, name string) (*plot.Plot, error) {
p.Y.Min = 0

// We are going to mutate values slice, so make a copy to avoid mangling
// underlying array and creating unexpected sideffect in caller's scope.
// underlying array and creating unexpected side-effect in caller's scope.
lValues := make([]float64, len(values))
copy(lValues, values)
// Make sure values are sorted
Expand Down Expand Up @@ -102,7 +102,7 @@ func CreateHistogramPlot(values []float64, name string) (*plot.Plot, error) {
p.Y.Label.Text = "N"

// We are going to mutate values slice, so make a copy to avoid mangling
// underlying array and creating unexpected sideffect in caller's scope.
// underlying array and creating unexpected side-effect in caller's scope.
lValues := make([]float64, len(values))
copy(lValues, values)

Expand Down Expand Up @@ -505,7 +505,7 @@ func horizontalLineWithLabel(y, xMin, xMax float64, label string) (*plotter.Line

// createQuantileLines is helper to create vertical Quantile lines.
func createQuantileLines(p *plot.Plot, values []float64, quantiles ...float64) []plot.Plotter {
var plotters []plot.Plotter
plotters := make([]plot.Plotter, 0, len(quantiles))
colorCount := len(ColorPalette)
for i, q := range quantiles {
qVal := stat.Quantile(q, stat.Empirical, values, nil)
Expand Down
2 changes: 1 addition & 1 deletion internal/analysis/plot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ var frameMetricsFile = "../../testdata/vqm/frame_metrics.json"

// getVmafValues fixture provides slice of VMAF metrics.
func getVmafValues(t *testing.T) []float64 {
var values []float64
var metrics vqm.FrameMetrics

j, err := os.Open(frameMetricsFile)
Expand All @@ -32,6 +31,7 @@ func getVmafValues(t *testing.T) []float64 {
err2 := json.NewDecoder(j).Decode(&metrics)
require.NoError(t, err2, "Error Unmarshaling metrics")

values := make([]float64, 0, len(metrics))
for _, v := range metrics {
values = append(values, v.VMAF)
}
Expand Down
22 changes: 7 additions & 15 deletions internal/vqm/vqm.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,6 @@ var DefaultFfmpegVMAFTemplate = "-hide_banner -i {{.CompressedFile}} -i {{.Sourc
"-lavfi libvmaf=n_subsample=1:log_path={{.ResultFile}}:feature=name=psnr:" +
"log_fmt=json:model=path={{.ModelPath}}:n_threads={{.NThreads}} -f null -"

// Measurer is an interface that must be implemented by VQM tool which is capable of
// calculating Vide Quality Metrics.
type Measurer interface {
// Measure should run actual VQM measuring process
Measure() error
GetMetrics() (*AggregateMetric, error)
}

// FfmpegVMAFConfig exposes parameters for ffmpegVMAF creation.
type FfmpegVMAFConfig struct {
FfmpegPath string
Expand All @@ -45,8 +37,8 @@ type FfmpegVMAFConfig struct {
}

// NewFfmpegVMAF will initialize VQM Measurer based on ffmpeg and libvmaf.
func NewFfmpegVMAF(cfg *FfmpegVMAFConfig, compressedFile, sourceFile string) (Measurer, error) {
var vqt *ffmpegVMAF
func NewFfmpegVMAF(cfg *FfmpegVMAFConfig, compressedFile, sourceFile string) (*FfmpegVMAF, error) {
var vqt *FfmpegVMAF

// Too much CPU threads are also bad. This was an issue on 128 threaded AMD
// EPYC, ffmpeg was deadlocking at some point during VMAF calculations.
Expand Down Expand Up @@ -82,7 +74,7 @@ func NewFfmpegVMAF(cfg *FfmpegVMAFConfig, compressedFile, sourceFile string) (Me
return vqt, fmt.Errorf("NewFfmpegVMAF() prepare command: %w", err)
}

vqt = &ffmpegVMAF{
vqt = &FfmpegVMAF{
exePath: cfg.FfmpegPath,
ffmpegArgs: ffmpegArgs,
sourceFile: sourceFile,
Expand All @@ -95,8 +87,8 @@ func NewFfmpegVMAF(cfg *FfmpegVMAFConfig, compressedFile, sourceFile string) (Me
return vqt, nil
}

// ffmpegVMAF defines VQM tool and implements Measurer interface.
type ffmpegVMAF struct {
// FfmpegVMAF defines VQM tool and implements Measurer interface.
type FfmpegVMAF struct {
// Path to ffmpeg executable
exePath string
// ffmpeg command arguments
Expand All @@ -111,7 +103,7 @@ type ffmpegVMAF struct {
measured bool
}

func (f *ffmpegVMAF) Measure() error {
func (f *FfmpegVMAF) Measure() error {
var err error

if f.measured {
Expand Down Expand Up @@ -160,7 +152,7 @@ type Metric struct {
Variance float64
}

func (f *ffmpegVMAF) GetMetrics() (*AggregateMetric, error) {
func (f *FfmpegVMAF) GetMetrics() (*AggregateMetric, error) {
if !f.measured {
return nil, errors.New("GetMetrics() depends on Measure() called first")
}
Expand Down
11 changes: 3 additions & 8 deletions internal/vqm/vqm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,8 @@ import (
// go test -run ^TestFfmpegVMAF ./internal/vqm -save-result
var saveResultFile = flag.Bool("save-result", false, "Save result file")

func TestFfmpegVMAFImplementsMeasurer(t *testing.T) {
// Test that tool implements Measurer interface.
var _ Measurer = &ffmpegVMAF{}
}

func TestFfmpegVMAF(t *testing.T) {
var tool Measurer // tool under test
var tool *FfmpegVMAF // tool under test
var aggMetrics *AggregateMetric

wrkDir := t.TempDir()
Expand Down Expand Up @@ -106,7 +101,7 @@ func TestFfmpegVMAF_Negative(t *testing.T) {
libvmafModelPath, _ := tools.FindLibvmafModel()

// Valid tool fixture.
getValidTool := func() Measurer {
getValidTool := func() *FfmpegVMAF {
srcFile := "../../testdata/video/testsrc01.mp4"
compressedFile := "../../testdata/video/testsrc01.mp4"
resultFile := t.TempDir() + "/result.json"
Expand All @@ -123,7 +118,7 @@ func TestFfmpegVMAF_Negative(t *testing.T) {
}

// Invalid tool fixture.
getInvalidTool := func() Measurer {
getInvalidTool := func() *FfmpegVMAF {
srcFile := "nonexistent-source"
compressedFile := "non-existent-compressed"
resultFile := t.TempDir() + "/result.json"
Expand Down
13 changes: 5 additions & 8 deletions run.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
"github.com/jszwec/csvutil"
)

// CreateRunCommand will create Commander instance from App.
func CreateRunCommand() Commander {
// CreateRunCommand will create instance of App.
func CreateRunCommand() *App {
longHelp := `Subcommand "run" will execute encoding plan according to definition in file
provided as parameter to -plan flag and will calculate and report VQM
metrics. This flag is mandatory.
Expand All @@ -50,9 +50,6 @@ Examples:
return app
}

// Make sure App implements Commander interface.
var _ Commander = (*App)(nil)

// App is subcommand application context that implements Commander interface.
type App struct {
// Configuration object
Expand Down Expand Up @@ -324,9 +321,9 @@ func (a *App) analyse() error {

// saveReport writes recorded metrics to report file.
func (a *App) saveReport() error {
var report []metric.Record

for _, id := range a.mStore.GetIDs() {
ids := a.mStore.GetIDs()
report := make([]metric.Record, 0, len(ids))
for _, id := range ids {
r, err := a.mStore.Get(id)
if err != nil {
return fmt.Errorf("getting record (id=%v) from metric store: %w", id, err)
Expand Down
7 changes: 2 additions & 5 deletions vqmplot.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import (
// Support these metrics for plotting.
var supportedMetrics = "VMAF, PSNR, MS-SSIM"

// CreateVQMPlotCommand will create Commander instance from VQMPlotApp.
func CreateVQMPlotCommand() Commander {
// CreateVQMPlotCommand will create instance of VQMPlotApp.
func CreateVQMPlotCommand() *VQMPlotApp {
longHelp := `Subcommand "vqmplot" will create plot for given metric from JSON report as
generated by libvmaf.
Expand All @@ -46,9 +46,6 @@ Examples:
return app
}

// Make sure VQMPlotApp implements Commander interface.
var _ Commander = (*VQMPlotApp)(nil)

// VQMPlotApp is vqmplot subcommand context that implements Commander interface.
type VQMPlotApp struct {
// FlagSet instance
Expand Down

0 comments on commit b8e53a1

Please sign in to comment.