-
Notifications
You must be signed in to change notification settings - Fork 316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make all ffmpeg options configurable. #560
Conversation
Moves the configuration of ffmpeg into the config file, allowing to change the input, codec, or any of the options at runtime.
These now come indirectly from the config file.
"-thread_queue_size", ${jibri.ffmpeg.queue-size} | ||
"-i", ":0.0+0,0" | ||
"-f", ${jibri.ffmpeg.audio-source} | ||
"-thread_queue_size", ${jibri.ffmpeg.queue-size} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have thread_queue_size listed twice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's for the different sources (-i), it's been like this in jibri:
https://github.com/jitsi/jibri/blob/0271286b4c9dfbe5807a835bd1244e919be47f39/src/main/kotlin/org/jitsi/jibri/capture/ffmpeg/Commands.kt#L28C9-L28C35
src/main/resources/reference.conf
Outdated
"-vsync", "2" | ||
] | ||
// Audio encoding parameters | ||
audio-params = [ "-acodec", "libopus", "-b:a", "64k" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Opus widely-enough supported in video players?
|
||
companion object { | ||
val recordingFormat: String by config("jibri.ffmpeg.recording-format".from(Config.configSource)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is both the argument to '-f' in ffmpeg and also the file extension? Do those always match?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, for example ffmpeg uses 'matroska' for the muxer but the is usually '.mkv'. I figured the extension is not important and I'd rather have the format configurable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could have them be two separate config parameters, with one value defaulting to the other one.
// Video encoding parameters specific to file recording | ||
video-params-recording = ${jibri.ffmpeg.video-params} [ | ||
"-profile:v", "main" | ||
"-level", "3.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you specify a profile and level for recording but not for streaming?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea, I just ported the existing flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
override val options: Array<String> = arrayOf( |
override val options: Array<String> = arrayOf( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok; while changing/rationalizing these is probably a good idea, we should keep it as a separate PR to avoid being confusing.
This makes all ffmpeg options configurable in jibri.conf. The default audio codec is switched to opus. The "-tune zerolatency" and "-g 60" parameters are enabled only for live-streaming (lowering encoding complexity for file recording)