Skip to content

Commit

Permalink
Fix dump-conf to always return configuration
Browse files Browse the repository at this point in the history
Previously in case of missing ffmpeg, ffprobe or VMAF model files
auto-detection mechanism would end execution and there was no chance to
mend situation even with -conf flag and working configuration file.

Now auto-detecting mechanism for default configuration will list "not
found" and configuration verification will catch the issue. Upside being
that one can get a rough configuration on systems with unconventional
tool locations, which can be used as a basis for configuration file.
  • Loading branch information
jurisevo committed Sep 19, 2024
1 parent 43b4bcb commit 7f0fdba
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 15 deletions.
15 changes: 6 additions & 9 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,25 +95,25 @@ func (c *Config) OverrideFrom(src Config) {
//
// For some configuration options a default value will be specified, for others an
// auto-detection mechanism will populate option values.
func loadDefaultConfig() (Config, error) {
func loadDefaultConfig() Config {
var cfg Config

// For default configuration attempt to locate ffmpeg binary.
ffmpeg, err := tools.FfmpegPath()
if err != nil {
return cfg, fmt.Errorf("DefaultConfig: %w", err)
ffmpeg = "not found"
}

// For default configuration attempt to locate ffprobe binary.
ffprobe, err := tools.FfprobePath()
if err != nil {
return cfg, fmt.Errorf("DefaultConfig: %w", err)
ffprobe = "not found"
}

// For default configuration attempt to locate VMAF model file.
libvmafModel, err := tools.FindLibvmafModel()
if err != nil {
return cfg, fmt.Errorf("DefaultConfig: %w", err)
libvmafModel = "not found"
}

cfg = Config{
Expand All @@ -124,7 +124,7 @@ func loadDefaultConfig() (Config, error) {
ReportFileName: NewConfigVal(defaultReportFile),
}

return cfg, nil
return cfg
}

// loadConfigFromFile will load configuration from file.
Expand All @@ -144,10 +144,7 @@ func loadConfigFromFile(f string) (cfg Config, err error) {
// function to use for config loading. Configuration file is optional e.g. can be "".
func LoadConfig(configFile string) (cfg Config, err error) {
// Initialize default configuration.
cfg, err = loadDefaultConfig()
if err != nil {
return cfg, err
}
cfg = loadDefaultConfig()

// Load configuration from file and override default configuration options.
if configFile != "" {
Expand Down
27 changes: 21 additions & 6 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,16 @@ import (
)

func Test_loadDefaultConfig(t *testing.T) {
c, err := loadDefaultConfig()
assert.NoError(t, err, "Should create DefaultConfig without errors")

c := loadDefaultConfig()
assert.NoError(t, c.Verify(), "DefaultConfig should be valid")
}

func Test_loadDefaultConfig_Negative(t *testing.T) {
// Messing up PATH should result in failure detecting ffmpeg and ffprobe which
// should result in error from calling DefaultConfig().
t.Setenv("PATH", "")
_, err := loadDefaultConfig()
assert.ErrorContains(t, err, "DefaultConfig: ")
c := loadDefaultConfig()
assert.Error(t, c.Verify())
}

func Test_loadConfigFile(t *testing.T) {
Expand Down Expand Up @@ -153,7 +151,6 @@ func Test_Config_OverrideFrom(t *testing.T) {
}

func Test_DumpConfApp_Run(t *testing.T) {
commandOutput := &bytes.Buffer{}
// This is one option we try to make sure is in dumped config file.
want := `"report_file_name": "test_report.json"`

Expand All @@ -166,10 +163,28 @@ func Test_DumpConfApp_Run(t *testing.T) {
cmd := CreateDumpConfCommand()

// Redirect output to buffer
commandOutput := &bytes.Buffer{}
cmd.out = commandOutput

err := cmd.Run([]string{"-conf", confFile})
assert.NoError(t, err, "Unexpected error running encode")
// Check that config dump contains options we specified in config file.
assert.Contains(t, commandOutput.String(), want)
}

func Test_DumpConfApp_Run_WithNotFound(t *testing.T) {
// This will make ffmpeg and ffprobe auto-detect to fail.
t.Setenv("PATH", "")

// Run command will generate encoding artifacts and analysis artifacts.
cmd := CreateDumpConfCommand()

// Redirect output to buffer
commandOutput := &bytes.Buffer{}
cmd.out = commandOutput

err := cmd.Run([]string{})
assert.Contains(t, commandOutput.String(), `"ffmpeg_path": "not found"`)
assert.Contains(t, commandOutput.String(), `"ffprobe_path": "not found"`)
assert.ErrorContains(t, err, "configuration validation")
}

0 comments on commit 7f0fdba

Please sign in to comment.