Update args builder to respect WithAudio

[rafiramadhana]

Resolves https://github.com/mauricioabreu/mosaic-video/issues/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