%ffmpeg channels can't take variable

[simon1tan]

Describe the bug
Getting an error when I try to use a variable to set channels. Feels like a bug. I apologize is I am writing this wrong.

channels = "2"
bitrate = "128"
samplerate = "44100"

# This works
%ffmpeg(format="mp3",%audio(codec="libmp3lame", channels=2.0, b=bitrate^"k", ar=int_of_string(samplerate) ))

# This fails:
%ffmpeg(format="mp3",%audio(codec="libmp3lame", channels=int_of_string(channels), b=bitrate^"k", ar=int_of_string(samplerate )))

At line 7, char 0-128:

Error 12: Unsupported format: %ffmpeg(format="mp3", %audio(codec="libmp3lame", channels=int_of_string(channels), b=^(bitrate,"k"), ar=int_of_string(samplerate))).
You must be missing an optional dependency.

Expected behavior
I expect the second one to work just like the first since

Version details

• OS: Windows
• Version 2.1.1

Install method
Github release

[smimram]

This is actually a "feature": we use the number of channels in the format to make sure that we have the right number of channels in the source. Of course, we could handle cases like this where the value is constant, but this quickly gets messy (what if channel is the result of some complicated computation?). Bitrate and samplerate are not checked (since we use the frame default for the samplerate) and therefore we accept any value.

Is this really bad for you?

We should at least improve the error message...

[simon1tan]

thanks for your reply. I am not understanding as I am setting the channel to a valid value, just using a variable to do it. I guess you are saying you disallow using a variable to set the channel to prevent bad source but a user could still mistakenly set the channel wrong by channels=0. I don't agree with hand holding that prevents something that can be very legit. I'd rather see a warning message instead and when it fails, it is entirely my fault for not heeding the warning. And I'll see it in the logs so I know what needs to be fixed without having to write in. Thanks for all your help and the work you put in.

[smimram]

The situation is not exactly "we disallow variables because we don't like it". The startup of a Liquidsoap script takes place in two phases

1. first we check that all the types match (i.e. we don't have a string where an int is expected, but also that the number of channels for sources match)
2. we run the program

In the first phase, we use a standard typechecking algorithm where nothing is known about the value of the variables (only their type). My comment was stating that in principle we should be able to propagate constants at this phase, but this would be non-trivial and would clutter the code, so that I'd rather avoid that unless this is bringing useful improvement. (and here, I don't think that this is worth the pain...)