feat!: Unify ReplayGain handling and calculation

[vitoyucepi]

Summary

This PR unifies ReplayGain handling by:

• A new metadata.replaygain method extracts a unified replay gain value in dB from metadata, handling r128_track_gain and replaygain_track_gain internally to provide a single gain value.

• Adding a compute parameter to file.replaygain and enable_replaygain_metadata to control gain calculation when metadata is missing replaygain tags. enable_replaygain_metadata calls file.replaygain internally with the compute value.

• Removing the unnecessary ebu_r128 parameter from replaygain since EBU R128 data will be extracted from metadata when available.

Breaking changes

• replaygain no longer has the ebu_r128 named parameter. r128_track_gain is now always processed from metadata in the metadata.replaygain function.
• metadata.replaygain extracts gain from replaygain_track_gain always in dB.

Additional

Closes #3435.
Requires #3478.

[vitoyucepi]

I need some help with the tests for replaygain, file.replaygain.compute, and file.replaygain.

[smimram]

Looks fine thanks!

[toots]

Hi! Do you need anything more with this PR? If @smimram has approved it I trurst y'all 🙂

[vitoyucepi]

Do you need anything more with this PR?

I need some help with the tests for replaygain, file.replaygain.compute, and file.replaygain.

How do I plan to test file.replaygain

1. mp3 with replaygain_track_gain.
2. mp3 with r128_track_gain.
3. mp3 with both replaygain_track_gain and r128_track_gain.
4. opus with replaygain_track_gain.
5. opus with r128_track_gain.
6. opus with both replaygain_track_gain and r128_track_gain.
7. mp3 with replaygain_track_gain and compute=true.
8. mp3 with replaygain_track_gain and compute=false.
9. mp3 without replaygain_track_gain and compute=true.
10. mp3 without replaygain_track_gain and compute=false.

[toots]

Okay. Do you mean you would like some support writing the test?

[vitoyucepi]

Do you mean you would like some support writing the test?

Yes, I do!

1. Where should I place tests?
2. How to generate test assets.
   I can create them before testing the corresponding function.
   ffmpeg -f lavfi -i 'anullsrc=r=44100:cl=mono' -t 0.01 -c:a libmp3lame -metadata REPLAYGAIN_TRACK_GAIN='10 dB' -y '1.mp3'.
3. If creating files at test time is not a good idea, how do I create minimal files?
4. How do I store them in the project, as files or as base64?

[toots]

Do you mean you would like some support writing the test?

Yes, I do!

Sorry for the delay.

1. Where should I place tests?

I think the pattern we seemed to have been following mostly is:

• tests/media for everything encoder/decoder/media content test
• tests/stream for everything operator etc.

Thus, I would say tests/stream.

2. How to generate test assets.
   I can create them before testing the corresponding function.
   ffmpeg -f lavfi -i 'anullsrc=r=44100:cl=mono' -t 0.01 -c:a libmp3lame -metadata REPLAYGAIN_TRACK_GAIN='10 dB' -y '1.mp3'.

Yes, that's how we already do it. See for instance in tests/stream/dune:

(rule
 (alias citest)
 (target file1.mp3)
 (action
  (run
   ffmpeg
   -hide_banner
   -loglevel
   error
   -f
   lavfi
   -i
   "sine=frequency=220:duration=5"
   -ac
   2
   %{target})))

You can then put this file has a dependency of your test:

(rule
 (alias citest)
 (package liquidsoap)
 (deps
  195.liq
  ./file1.mp3
  ../../src/bin/liquidsoap.exe
  (package liquidsoap)
  (:test_liq ../test.liq)
  (:run_test ../run_test.exe))
 (action (run %{run_test} 195.liq liquidsoap %{test_liq} 195.liq)))

3. If creating files at test time is not a good idea, how do I create minimal files?

at test time works!

4. How do I store them in the project, as files or as base64?

at test time works!

To run a test, we use the tests/test.liq library and a test runner run_test.exe.

Once a new test is placed in tests/stream, you you dune:

% dune build @gendune --auto-promote

This will re-generate the dune files to include your test in the test suite.

If your test does not have generated test files, you can run it manually from within test/streams:

../../liquidsoap ../test.liq ./my-test.liq

If it does, the best is to run all the tests:

% dune build @citest

Once all the tests are ran, if your test still has issues you can manually go to _build/default/tests/streams and run it same as above, this time with all the files generated:

../../../../liquidsoap ../test.liq ./my-test.liq

If you edit the file inside _build/default/tests/streams, don't forget to copy it back to tests/streams!

Hope this helps!

[toots]

@vitoyucepi I had to change the test runner configuration. If you push another commit, they should run this time.

[vitoyucepi]

@toots, I'd like to create another PR with additional test methods.

def test.not_equals(first, second)
def test.almost_equals(~places=7, first, second)
def test.not_almost_equals(~places=7, first, second)

I'm planning to replace

null.defined(g) and abs(null.get(g) - 7.37) < 0.05

with

test.not_equals(g, null())
test.almost_equals(g, 7.37)

[toots]

Sounds great! Unless you want to also convert a bunch of tests to use the new API, you could also introduce it here directly.

[vitoyucepi]

I've created a separate PR with the required test functions.
#3478

[vitoyucepi]

I think this PR is ready.
@toots, could you please check tests/stream/dune and tests/streams/gen_dune.ml?

[toots]

This looks really good sorry for the delay reviewing this. Setting it for auto-merge now!