5
\$\begingroup\$

The class that I've created for ffmpeg that derives from System.Diagnostics.Process. I'm looking for any help with arguments that can be added for more support. A little background, I built this to work with a solution I've created using NAudio and AForge. NAudio is used to record audio while AForge Displays Images from a webcam and saves the images. This class is then used to first combine the images to a video then combine the video and audio for a final result. Anyhow, here is the code.

using System;
using System.Diagnostics;
using System.Threading.Tasks;
namespace Webcam
{
 public sealed class FFMPEG_PROCESS : Process
 {
 private bool _createdWithOptions;
 private string _args;
 public static FFMPEG_PROCESS WithOptions() = new FFMPEG_PROCESS();
 private FFMPEG_PROCESS() { _createdWithOptions = true; }
 public FFMPEG_PROCESS(string args) { _args = args; }
 //Don't allow setting processes StartInfo
 public new ProcessStartInfo StartInfo
 {
 get => base.StartInfo = new ProcessStartInfo("ffmpeg.exe", _args)
 {
 UseShellExecute = false,
 WindowStyle = ProcessWindowStyle.Hidden,
 CreateNoWindow = true
 };
 } 
 
 public FFMPEG_PROCESS WithOutput(string value)
 {
 if (!_createdWithOptions) return this;
 _args += $" {value}"; return this;
 }
 public FFMPEG_PROCESS WithInput(string value)
 {
 if (!_createdWithOptions) return this;
 _args += $" -i {value}"; return this;
 }
 public FFMPEG_PROCESS WithAudioType(string value)
 {
 if (!_createdWithOptions) return this;
 _args += $" -c:a {value}"; return this;
 }
 public FFMPEG_PROCESS WithConcatList(string value)
 {
 if (!_createdWithOptions) return this;
 _args += $" -f concat -safe 0 -i {value}"; return this;
 }
 public FFMPEG_PROCESS WithVideoFilter(string value)
 {
 if (!_createdWithOptions) return this;
 _args += $" -vf {value}"; return this;
 }
 
 /// <summary>
 /// The CRF of x264 and x265 video
 /// </summary> 
 public FFMPEG_PROCESS WithConstantRateFactor(int value)
 {
 if (!_createdWithOptions) return this;
 _args += $" -crf {value}"; return this; 
 }
 public new Task<FFMPEG_PROCESS> Start()
 {
 //initialize our StartInfo
 var result = StartInfo;
 if (base.StartInfo.Arguments != result.Arguments) 
 throw new ArgumentException(nameof(StartInfo));
 if(!base.Start()) throw new Exception(nameof(Start)); 
 return Task.FromResult(this);
 } 
}
}

Examples of using the class.

var ffmpegProcess = await FFMPEG_PROCESS
.WithOptions()
.WithInput(input.mp4)
.WithInput(((IAudioRecorder)audio).Filename)
.WithAudioType("flac") 
.WithOutput(output.mp4)
.Start(); 
ffmpegProcess.WaitForExit();
var ffmpegProcess = await FFMPEG_PROCESS
.WithOptions()
.WithConcatList(frameList)
.WithVideoFilter("\"fps=30, format=yuv420p\"")
.WithConstantRateFactor(30)
.WithOutput(output.mp4)
.Start(); 
ffmpegProcess.WaitForExit();

New FFMPEG_PROCESS Additions

public FFMPEG_PROCESS WithNoVideo()
{
 if (!_createdWithOptions) return this;
 _args += " -vn"; return this;
}
public FFMPEG_PROCESS WithVideoFrameRate(int value)
{
 if (!_createdWithOptions) return this;
 _args += $" -r {value}"; return this;
}
asked Jan 12 at 10:16
\$\endgroup\$
1
  • \$\begingroup\$ The answers don't have to be related to c# only to arguments that can be passed to ffmpeg. Both short and long arguments are acceptable. Explanations are encouraged but not a prerequisite \$\endgroup\$ Commented Jan 16 at 14:34

4 Answers 4

7
\$\begingroup\$

Multiple statements in a single line

if (!_createdWithOptions) return this;
_args += $" -i {value}"; return this;

You can change your code to have a single return statement. That would lead to a more legible code IMHO. To do that you should use guard expression instead of early exit inside your WithXYZ methods.

if (_createdWithOptions)
{
 _args += $" -i {value}"; 
}
return this;

That's also maintenance friendly. If you want to add for example validation logic against the value then you don't have to restructure the method body. You just have to add the preliminary check logic before you start using the parameter. Something like this

if (_createdWithOptions)
{
 if (string.IsNullOrEmpty(value))
 {
 throw new ArgumentException("Please provide non-empty input", nameof(value));
 }
 _args += $" -i {value}"; 
}
return this;

Calling the same WithXYZ method multiple times

What would happen if you call the WithAudioType twice?

.WithAudioType("flac")
.WithAudioType("wav")

Most probably the application will fail at the Start call saying that you can't provide the -c:a flag multiple times. OR it does not fail just takes the first argument or the second one.

One way to tackle this problem is to check inside each With methods whether the args already contains the given flag or not

if (args.Contains("-r"))
{
 // Your options:
 // 1. leave it as it is
 // 2. overwrite with the new value
 // 3. throw an exception that it is an invalid configuration
}
else
{
 _args += $" -r {value}";
}

With this approach you still can't guarantee that WithInput and WithOutput were called.


To add a holistic check before you start to kick off the process, you should separate the builder and the executor.

var process = FFMPEG_PROCESS_BUILDER
.WithOptions()
.WithInput(input.mp4)
.WithInput(((IAudioRecorder)audio).Filename)
.WithAudioType("flac") 
.WithAudioType("wav") 
.WithOutput(output.mp4)
.Build(); // This could fail with invalid configuration
var ffmpegProcess = await process.Start();

With this approach you can check the individual flags correctness as well as the mandatory flags' presence.

answered Jan 14 at 9:39
\$\endgroup\$
3
  • \$\begingroup\$ the error is Unknown encoder 'wav' \$\endgroup\$ Commented Jan 14 at 23:25
  • \$\begingroup\$ You could however use .WithAudioType("wav -c:a pcm_s16le") to encode the audio in ipcm format however there are limited audio players that can read this. MPC-HC is one of few that can, even VLC complains of "VLC could not decode the format "ipcm" (no description for this codec)" \$\endgroup\$ Commented Jan 14 at 23:41
  • \$\begingroup\$ When .WithAudioType(type) is called consecutively it takes the last audio type specified. With one call before WithInput(input) and one before WithInput(output) is equivalent to WithInput(decode input) and WithInput(encode output) \$\endgroup\$ Commented Jan 15 at 17:30
6
\$\begingroup\$

You should follow the coding style as instructed here C# identifier naming rules and conventions. So, your class name should be Pascal Cased, with no underscore.

also this line :

public static FFMPEG_PROCESS WithOptions() = new FFMPEG_PROCESS();

would give you a single instance across the application. So, if the class is intended to have multiple instances, then they will override each other. So, you will need to remove static so you application can start a new instance for each file.

Here :

//Don't allow setting processes StartInfo
public new ProcessStartInfo StartInfo
{
 get => base.StartInfo = new ProcessStartInfo("ffmpeg.exe", _args)
 {
 UseShellExecute = false,
 WindowStyle = ProcessWindowStyle.Hidden,
 CreateNoWindow = true
 };
} 

still, StartInfo can be overridden, since in the base it has a public setter. So, this code does not prevent that. So, it seems the Process inheritance for this class is not needed, as you need to hide the process from the consumer, and handle it. Let the consumer just communicate with your class and its option, and do not give it access to the main engine (the process and its arguments).

in any case, what you are looking for is a builder pattern, so you need to move the arguments into its own class. This way, you can have more room to customize your arguments and handle them better. You can use Dictionary to better handling your arguments, and then just enumerate its values, excluding null, and then just do a simple string.Join(" ", args).Or you can go further and categorize them based on type (Audio, Video, ..etc.), where you can implement a base class where you define each argument, for a strongly-typed class (easier to maintain the arguments for the long run). Here is a pseudo code (from your example code) that how it might looks if you've gone wild :

var processA = new FfmpegProcess("input.mp4", "output.mp4")
 .WithAudioOptions()
 .WithAudioType(AudioType.Flac)
 .Build();
await processA.StartAsync();
var processB = new FfmpegProcess("input.mp4", "output.mp4")
 .WithVideoOptions()
 .ConcatList(frameList)
 .Fps(30)
 .Resolution(VideoResolution.Yuv420p)
 .ConstantRateFactor(30)
 .Build();
await processB.StartAsync();

here is one example that came in my mind :

public sealed class FfmpegProcess 
{
 private readonly string _output;
 private readonly string _input;
 private string _args;
 public FfmpegProcess(string inputFilePath, string outputFilePath)
 {
 ArgumentNullException.ThrowIfNullOrWhiteSpace(inputFilePath);
 ArgumentNullException.ThrowIfNullOrWhiteSpace(outputFilePath);
 if (!File.Exists(inputFilePath))
 throw new FileNotFoundException(nameof(inputFilePath));
 _input = inputFilePath;
 _output = outputFilePath;
 }
 public async Task StartAsync(CancellationToken cancellationToken = default)
 {
 ArgumentNullException.ThrowIfNullOrWhiteSpace(_args);
 try
 {
 var process = new ProcessStartInfo("ffmpeg.exe", _args)
 {
 UseShellExecute = false,
 WindowStyle = ProcessWindowStyle.Hidden,
 CreateNoWindow = true
 };
 using (var exe = Process.Start(process))
 {
 await exe.WaitForExitAsync(cancellationToken);
 }
 }
 catch (Exception ex)
 {
 // handle exceptions
 }
 }
}
public sealed class FfmpegOptionBuilder
{
 private readonly FfmpegProcess _instance;
 private readonly Dictionary<string, string> _args = new()
 {
 {"audio_type", null},
 {"video_filter", null}
 };
 public FfmpegOptionBuilder(FfmpegProcess instance) => _instance = instance;
 public FfmpegOptionBuilder AudioType(string value)
 {
 _args["audio_type"] = ValueOrThrowGetValue("-c:a", value);
 return this;
 }
 public FfmpegOptionBuilder VideoFilter(string value)
 {
 _args["video_filter"] = ValueOrThrowGetValue("-vf", value);
 return this;
 }
 private static string ValueOrThrowGetValue(string arg, string value)
 {
 ArgumentNullException.ThrowIfNullOrWhiteSpace(value);
 return string.Format("{0} {1}", arg, value);
 }
 public FfmpegProcess Build()
 {
 var args = _args.Values.Where(x => x != null);
 if (args.Any())
 _instance._args = string.Join(" ", args);
 return _instance;
 }
}
 
public static class FfmpegExtensions
{
 public static FfmpegOptionBuilder WithOptions(this FfmpegProcess process) => new(process);
}
answered Jan 14 at 17:01
\$\endgroup\$
5
  • \$\begingroup\$ In the OP's first sample code there are two WithInput calls: one for the video and another for the audio stream. As far as I can see your proposed solution can handle only a single input file. \$\endgroup\$ Commented Jan 14 at 20:32
  • \$\begingroup\$ @PeterCsala,, that's true, my solution can be adjusted as it fits, it's just a startup example that would give a clear view on my points. As for the inputs, it can be added in the constructor, or in the arguments, which is totally up to the OP. \$\endgroup\$ Commented Jan 14 at 21:37
  • \$\begingroup\$ The StartInfo can be overridden but as an override it's a {get;set;} and marking it new ensures that it's a {get;} only. Marking it as an override and not allowing it to be set is confusing and misleading to the end user. \$\endgroup\$ Commented Jan 19 at 5:04
  • \$\begingroup\$ @CharlesHenington avoid doing all that, and just use a private instance of Process instead. Keep the process hidden from the consumer, and gain control of it by exposing some options to the consumer where you use these options with your process internally. \$\endgroup\$ Commented Jan 19 at 8:37
  • \$\begingroup\$ public static FFMPEG_PROCESS WithOptions() = new FFMPEG_PROCESS(); is a method that returns a new instance on every call, public static FFMPEG_PROCESS WithOptions = new FFMPEG_PROCESS(); would be a single instance. \$\endgroup\$ Commented Jul 27 at 21:49
5
+50
\$\begingroup\$

Given that the WithX methods never do anything if _createdWithOptions is false, it seems like any program which attempts to use those methods in such situations is necessarily incorrect. In that case I think it'd be preferable if the WithX methods indicated this more clearly by throwing an InvalidOperationException instead of just silently doing nothing.

I'd also argue the code misbehaves in situations where an argument contains spaces, such as FFMPEG_PROCESS.WithOptions().WithOutput("my file.mp4"). Yes, the caller can quote their arguments, but since we're attempt to provide an abstractions that tries to mostly hide the command line argument management, that shouldn't be the caller's responsibility in my opinion.

answered Jan 20 at 23:56
\$\endgroup\$
0
\$\begingroup\$

From the chaining options provided we can convert an audio or video file to gsm using the ConvertToGSM method. In the ConvertToH265 method we can convert audio or video to H265 using the provided chaining options as well as using these custom chaining options as asked for WithAudioCopy, WithPreset and WithVideoEncoding. These chaining options are what is being asked in the question. If you simply post ffmpeg commands those are the equivalent for example using ConvertToH265 would be 'ffmpeg -i input.mp4 -c:v libx265 -crf 18 -preset slow -c:a copy output.mp4'

The expansion of chaining options adds more layer to the original code increasing the amount of options available. ConvertToGSM allows access to quick options for conversion to 'gsm'.

public FFMPEG_PROCESS WithPreset(string value)
{
 if (!_createdWithOptions) return this;
 _args += $" -preset {value}"; return this;
}
public FFMPEG_PROCESS WithVideoEncoding(string encoder)
{
 if (!_createdWithOptions) return this;
 _args += $" -c:v {encoder}"; return this; 
}
public FFMPEG_PROCESS WithAudioCopy()
{
 if (!_createdWithOptions) return this;
 _args += "-c:a copy";
 return this;
}
/// <summary>
/// Converts to GSM. 
/// </summary> 
public static async Task ConvertToGSM(string input, string output)
{
 var process = await WithOptions() //initialize FFMPEG_PROCESS
 .WithInput(input) //add the input
 .WithSampleRate(8000) //GSM const SampleRate is 8000
 .WithNoVideo() //no video
 .WithForcedFormat("gsm")//force encoding the output as gsm
 .WithOutput(output)//add the output 
 .WithOverwriteAlways() //overwrite the output if exists
 .Start();
 await process.WaitForExit();
}
 public static async Task ConvertToH265(string input, string output)
 {
 var process = await WithOptions()
 .WithInput(input) 
 .WithVideoEncoding("libx265")
 .WithConstantRateFactor(18) 
 .WithPreset("slow")
 .WithAudioCopy()
 .WithOutput(output)
 .Start();
 await process.WaitForExit();
 }
Peilonrayz
44.4k7 gold badges80 silver badges157 bronze badges
answered Jan 21 at 1:26
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.