Update args builder to respect WithAudio

[rafiramadhana]

Resolves #1

[mauricioabreu]

@rafiramadhana did you run the tests?
I've just set up a test workflow. Would you mind to run the tests locally and rebase your branch with the main branch?

[rafiramadhana]

@rafiramadhana did you run the tests?

I only run the package's unit test

[rafiramadhana]

I've just set up a test workflow. Would you mind to run the tests locally and rebase your branch with the main branch?

ok, I will take a look

[rafiramadhana]

@mauricioabreu

the test runs fine on my local

➜  mosaic-video git:(1-with-audio-field) ✗ go test -v ./...
?   	github.com/mauricioabreu/mosaic-video	[no test files]
?   	github.com/mauricioabreu/mosaic-video/cmd	[no test files]
?   	github.com/mauricioabreu/mosaic-video/internal/config	[no test files]
?   	github.com/mauricioabreu/mosaic-video/internal/locking	[no test files]
?   	github.com/mauricioabreu/mosaic-video/internal/logging	[no test files]
?   	github.com/mauricioabreu/mosaic-video/internal/mocks	[no test files]
?   	github.com/mauricioabreu/mosaic-video/internal/mosaic/command	[no test files]
?   	github.com/mauricioabreu/mosaic-video/internal/uploader	[no test files]
=== RUN   TestBuildFFMPEGCommand
=== RUN   TestBuildFFMPEGCommand/Multiple_URLs_with_audio
=== RUN   TestBuildFFMPEGCommand/Multiple_URLs_without_audio
--- PASS: TestBuildFFMPEGCommand (0.00s)
    --- PASS: TestBuildFFMPEGCommand/Multiple_URLs_with_audio (0.00s)
    --- PASS: TestBuildFFMPEGCommand/Multiple_URLs_without_audio (0.00s)
=== RUN   TestGenerateMosaic
--- PASS: TestGenerateMosaic (0.00s)
PASS
ok  	github.com/mauricioabreu/mosaic-video/internal/mosaic	0.004s
=== RUN   TestGenerateMosaicWhenLockingFails
--- PASS: TestGenerateMosaicWhenLockingFails (0.00s)
=== RUN   TestGenerateMosaicWhenExecutingCommandFails
--- PASS: TestGenerateMosaicWhenExecutingCommandFails (0.00s)
PASS
ok  	github.com/mauricioabreu/mosaic-video/internal/worker	(cached)

[mauricioabreu]

I see the problem @rafiramadhana

We are using ElementsMatch from assert. It is not a good option here because the order of the parameters matter when we talk about FFMPEG. I think we need to change the way we assert the command is right (parameters, order, etc)

[rafiramadhana]

i see, wdyt about this?

i think if we are not planning to add more args (frequently in the near future), this is good enough

args := []string{
	"-loglevel", "error",
	"-i", cfg.StaticsPath + "/background.jpg",
	"-i", m.Medias[0].URL,
	"-i", m.Medias[1].URL,
	"-filter_complex", filterComplex,
	"-map", "[mosaico]",
}

if m.WithAudio {
	args = append(args, "-map", "1:a")
}

args = append(args, []string{
	"-c:v", "libx264",
	"-x264opts", "keyint=30:min-keyint=30:scenecut=-1",
	"-preset", "ultrafast",
	"-threads", "0",
	"-r", "24",
	"-c:a", "copy",
	"-f", "hls",
	"-hls_playlist_type", "event",
	"-hls_time", "5",
	"-strftime", "1",
	"-hls_segment_filename", segmentPattern,
	"-method", "PUT",
	"-http_persistent", "1",
	"-sc_threshold", "0",
	fmt.Sprintf("http://localhost:8080/%s", playlistPath),
}...)

return args

for the unit test, I will update them to also assert the args ordering

[mauricioabreu]

@rafiramadhana yes, this is good enough.

[rafiramadhana]

@rafiramadhana yes, this is good enough.

ready for review

[mauricioabreu]

@rafiramadhana thank you!

[mauricioabreu]

@rafiramadhana tests are failing. Could you check why?

[rafiramadhana]

@rafiramadhana tests are failing. Could you check why?

updated, ready to review

[mauricioabreu]

@rafiramadhana tests are still failing as you can see in the logs. Could you check why?

[rafiramadhana]

@rafiramadhana tests are still failing as you can see in the logs. Could you check why?

ah sry, i've pushed the fix. thanks!

[mauricioabreu]

Thank you @rafiramadhana
merged