tomaszzmuda / Xabe.FFmpeg

.NET Standard wrapper for FFmpeg. It allows to process media without know how FFmpeg works, and can be used to pass customized arguments to FFmpeg from dotnet core application.
https://xabe.net/product/xabe_ffmpeg/
Other
715 stars 128 forks source link

Snippets fall in a deadlock #435

Closed HenningF2 closed 1 year ago

HenningF2 commented 1 year ago

When I used the snippets, I recognized that only one of them worked for me (Concatenate). All others do not react after the call. It looks like a deadlock.

I checked the code and found the following: public async Task Concatenate(string output, params string[] inputVideos) { return await Conversion.Concatenate(output, inputVideos); }

and for example public async Task ExtractAudio(string inputPath, string outputPath) { return await Task.FromResult(Conversion.ExtractAudio(inputPath, outputPath)); } Concatenate is the only function that does not have the "Task.FromResult" function and is the only one that works. I am new in async programming, but maybe it helps an expert to find out what is going wrong here,

tomaszzmuda commented 1 year ago

Hello @HenningF2 Can you show me how do you run those functions in your code?

HenningF2 commented 1 year ago

I use it in an eventhandler like this: private async void extractToolStripMenuItem_Click(object sender, EventArgs e) here I get the file name from user input "input" (It is a one minute mp4 file.)

then I take the code from the documentation: string output = Path.ChangeExtension(input , "mp3"); var conversion = FFmpeg.Conversions.FromSnippet.ExtractAudio(input, output); await conversion.Start();

This code does not compile. Error message is: Cannot wait for void. I changed the line to var conversion = await FFmpeg.Conversions.FromSnippet.ExtractAudio(arInputs[0], output); Now the program run, but stops at this line.

tomaszzmuda commented 1 year ago

Hmmm

There may be a problem with method definition: private async void extractToolStripMenuItem_Click(object sender, EventArgs e) Method should be marked as "async Task" because without that exceptions are handled differently. I'm not sure if you can do that in event handler. Generally, best idea is to separate that long running code from events code.

Also can you try to use those inputs and run ffmpeg directly? Maybe it is taking long time. And tell me please if ffmpeg process starts in background (you can check that in Task Manager or top command)

HenningF2 commented 1 year ago

I put the call in a separate function marked as "async Task(IConversion)". The result is the same. FFmpeg does not start in the background. The program never executes the line: "await conversion.Start();"

If I add the parameters and run ffmpeg directly everything works fine!!

If I look in ffmpegFacade.cs, I see that all functions have a line like: return await Task.FromResult(Conversion.ExtractAudio(..., ...)); except one which has the following: return await Conversion.Concatenate(output, inputVideos); and this one works.

If all which do not work look the same and the only that works looks difference, maybe the difference is the reason.

What will happen if one deletes the "Task.FromResult(....)?

tomaszzmuda commented 1 year ago

438 I'm going to get rid of those Tasks.FromResult

Can you try to use code directly from that branch and check if it solve the problem for you?

HenningF2 commented 1 year ago

I tried it, but it also does not work. My code to test it is pretty simple. It is just a form with one eventhandler:

using Xabe.FFmpeg; namespace TestExtractAudio { public partial class Form1 : Form { public Form1() { InitializeComponent(); }

    private async void extractToolStripMenuItem_Click(object sender, EventArgs e)
    {
        if(dlgOpen.ShowDialog() == DialogResult.OK)
        {
            await Extractit(dlgOpen.FileName);
        }
    }

    private async Task Extractit(string input)
    {
        string output = Path.ChangeExtension(input, ".mp3");
        var conversion = await FFmpeg.Conversions.FromSnippet.ExtractAudio(input, output);
        await conversion.Start();
        MessageBox.Show("Fertig!!");
    }
}

} I am using .net 6.0. The program stops at this line: var conversion = await FFmpeg.Conversions.FromSnippet.ExtractAudio(input, output); it never reaches: await conversion.Start();

This is my first experience with "async await". It looks pretty simple for the programmer, but it can be pretty complex in the background when you have a lot of tasks. It is difficult to debug (at least with my Visual Studio Community version). As I understand the call FFmeg.Conversions.FromSnippet.ExtractAudio just creates the parameter string for ffmpeg. Why has this to be asynchron? Mixing synchron and asynchron is not a good solution what I read, but if there is only one long running job (Conversion.Start()), wouldn't it much simpler to do only this in a separate thread?

tomaszzmuda commented 1 year ago

Please try to update your method to that: private async Task Extractit(string input) { try { string output = Path.ChangeExtension(input, ".mp3"); var conversion = await FFmpeg.Conversions.FromSnippet.ExtractAudio(input, output); await conversion.Start(); MessageBox.Show("Fertig!!"); } catch (Exception ex) { //Put breakpoint here } }

I think that you got exception underneath and because you event handler returns void not Task you don't know about that.

Unfortunatelly some methods needs to be asynchronous and all exposed methods of Xabe.FFmpeg return Task and should be awaited

HenningF2 commented 1 year ago

As before, the program stops at: await FFmpeg.Conversions.FromSnippet.ExtractAudio(input, output); It does not return to the catch clause. My understanding is that there are at least two tasks waiting for eachother. Waiting is not error for an Task. This explains that there is no exemption thrown.

HenningF2 commented 1 year ago

I tried the "solution" proposed in #352 and it works.

   private async Task Extractit(string input)
    {
            string output = Path.ChangeExtension(input, ".mp3");

            await Task.Run(async () => {
                var conversion = await FFmpeg.Conversions.FromSnippet.ExtractAudio(input, output);
                await conversion.Start();
            });

            MessageBox.Show("Fertig!!");
    }
HenningF2 commented 1 year ago

It looks like the Snippet is running on the UI thread. With "Task.Run" it runs in its own thread, but you need to use invoke to access UI controls on the form. This code works:

    private async Task Extractit(string input)
    {
            string output = Path.ChangeExtension(input, ".mp3");
            await Task.Run(async () => {
                var conversion = await FFmpeg.Conversions.FromSnippet.ExtractAudio(input, output);
                conversion.OnProgress += (sender, args) =>
                {
                    var percent = (int)(Math.Round(args.Duration.TotalSeconds / args.TotalLength.TotalSeconds, 2) * 100);
                    // update value of progress bar in UI thread
                    pgBar.Invoke(new Action(() => pgBar.Value = percent));
                 };
                await conversion.Start();
            });
            MessageBox.Show("Fertig!!");
    }
tomaszzmuda commented 1 year ago

I think that issue is strongly connected with GUI. Can you try to run Start method with .ConfigureAwait(false) without any Task.Run?

HenningF2 commented 1 year ago

With async and await the code is still running on the GUI thread (can be seen in the debugger window). ConfigureAwait(false) means the program should not continue on this thread, but take a new one from the threadpool. Looks strange for me. I tested your proposal: the result is as before the program stops at var conversion = await FFmpeg.Conversions.FromSnippet.ExtractAudio(input, output); To let ffmpeg run on a separate thread looks reasonable for me and it works!!

tomaszzmuda commented 1 year ago

Great. I'm closing that issue but a lot of people has problem with that so I'm going to change a bit code to not hang without Task.Run.