Skip to content

Commit

Permalink
feat: better error messages for ffmpeg process (#11)
Browse files Browse the repository at this point in the history
* feat: better error messages for ffmpeg process

* tests: add test cases for process errors

* fix test case error matching
  • Loading branch information
connerdouglass authored Jan 24, 2024
1 parent 9fc024a commit b6bb165
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 34 deletions.
7 changes: 7 additions & 0 deletions ffmpeg_arger.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,10 @@ package spireav
type FfmpegArger interface {
FfmpegArgs() ([]string, error)
}

// FfmpegArgs is a simple type that implements the FfmpegArger interface.
type FfmpegArgs []string

func (args FfmpegArgs) FfmpegArgs() ([]string, error) {
return args, nil
}
8 changes: 8 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
module github.com/spiretechnology/spireav

go 1.20

require github.com/stretchr/testify v1.8.4

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
10 changes: 10 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
62 changes: 28 additions & 34 deletions process.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type Process struct {
FfmpegArger FfmpegArger
EstimatedLengthFrames int
SysProcAttr *syscall.SysProcAttr
errorOutput string
}

// GetCommandString is a utility function that gets the FFmpeg command string that is run by this process
Expand All @@ -40,11 +41,10 @@ func (p *Process) Run(
ctx context.Context,
chanProgress chan<- Progress,
) error {

// Generate the FFmpeg arguments
args, err := p.FfmpegArger.FfmpegArgs()
if err != nil {
return err
return fmt.Errorf("generating ffmpeg args: %w", err)
}

// Create the command
Expand All @@ -53,48 +53,44 @@ func (p *Process) Run(

// If progress needs to be reported
if chanProgress != nil {

// Get a readable pipe of the command stdout
stderr, err := cmd.StderrPipe()
if err != nil {
return err
return fmt.Errorf("acquiring stderr pipe: %w", err)
}
defer stderr.Close()

// Parse the output into progress reports on the channel
go p.reportFFmpegProgress(
chanProgress,
stderr,
)

go p.reportFFmpegProgress(chanProgress, stderr)
}

// Start running the command in the background
if err := cmd.Start(); err != nil {
return err
return fmt.Errorf("starting ffmpeg process: %w", err)
}

// Wait for the process to end
if err := cmd.Wait(); err != nil {

// If the context triggered the process to be killed, we want to see the context's error
// instead of the process's error
if ctx != nil && ctx.Err() != nil {
return ctx.Err()
}
return err

switch e := err.(type) {
case *exec.ExitError:
return fmt.Errorf("ffmpeg exit code %d: %s", e.ExitCode(), p.errorOutput)
default:
return fmt.Errorf("ffmpeg error: %s: %s", err, p.errorOutput)
}
}
return nil

}

// RunWithProgress executes the process and reports progress back to a progress handler function
func (p *Process) RunWithProgress(
ctx context.Context,
progressFunc func(Progress),
) error {

// Create a channel for progress reporting
chanProgress := make(chan Progress)

Expand All @@ -115,14 +111,9 @@ func (p *Process) RunWithProgress(

// Return the error, if any
return err

}

func (p *Process) reportFFmpegProgress(
chanProgress chan<- Progress,
processOutput io.Reader,
) {

func (p *Process) reportFFmpegProgress(chanProgress chan<- Progress, processOutput io.Reader) {
// Calculate the start time
startTime := time.Now()

Expand All @@ -137,22 +128,26 @@ func (p *Process) reportFFmpegProgress(
scanner := bufio.NewScanner(processOutput)
scanner.Split(scanLinesWithCR)
for scanner.Scan() {

// Read a line from the input
line := scanner.Text()
line = strings.ReplaceAll(line, "\r", "\n")

// Read a new progress value
newProgress := parseProgressLine(
line,
p.EstimatedLengthFrames,
startTime,
)
if newProgress != nil {
progress = newProgress
chanProgress <- *progress
lines := strings.Split(line, "\n")
for _, line := range lines {
if line != "" {
p.errorOutput = line
}

// Read a new progress value
newProgress := parseProgressLine(
line,
p.EstimatedLengthFrames,
startTime,
)
if newProgress != nil {
progress = newProgress
chanProgress <- *progress
}
}

}

// Send the 100% progress
Expand All @@ -168,7 +163,6 @@ func (p *Process) reportFFmpegProgress(

// Close the channel
close(chanProgress)

}

func scanLinesWithCR(data []byte, atEOF bool) (advance int, token []byte, err error) {
Expand Down
37 changes: 37 additions & 0 deletions process_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package spireav_test

import (
"context"
"testing"

"github.com/spiretechnology/spireav"
"github.com/stretchr/testify/require"
)

func overrideFfmpegPath(path string) func() {
original := spireav.FfmpegPath
spireav.FfmpegPath = path
return func() {
spireav.FfmpegPath = original
}
}

func TestProcessErrors(t *testing.T) {
t.Run("invalid args", func(t *testing.T) {
proc := &spireav.Process{
FfmpegArger: spireav.FfmpegArgs{"several", "bad", "args"},
}
err := proc.RunWithProgress(context.Background(), func(p spireav.Progress) {})
require.Error(t, err, "expected error when running process with invalid args")
require.ErrorContains(t, err, "Invalid argument")
})
t.Run("ffmpeg not in path", func(t *testing.T) {
defer overrideFfmpegPath("something-that-does-not-exist")()
proc := &spireav.Process{
FfmpegArger: spireav.FfmpegArgs{"-i", "something"},
}
err := proc.RunWithProgress(context.Background(), func(p spireav.Progress) {})
require.Error(t, err, "expected error when path is missing ffmpeg binary")
require.ErrorContains(t, err, "starting ffmpeg process: exec: \"something-that-does-not-exist\": executable file not found")
})
}

0 comments on commit b6bb165

Please sign in to comment.