ffmpeginteropx / FFmpegInteropX

FFmpeg decoding library for Windows 10 UWP and WinUI 3 Apps
Apache License 2.0
212 stars 53 forks source link

It always crashes after resetting the Source of MediaPlayer in UWP. #314

Closed yikuo123 closed 1 year ago

yikuo123 commented 2 years ago

It always crashes after resetting the Source of MediaPlayer in UWP.

FFmpegInteropX version: 5.1.100 FFmepgInteropX.UWP version: 1.0.1

Code as following:

string url = "https://samples.mplayerhq.hu/Matroska/subtitles/multiple_sub_sample.mkv";

FFmpegInteropX.FFmpegMediaSource streamMediaSource = await FFmpegInteropX.FFmpegMediaSource.CreateFromUriAsync(url);
streamMediaPlayer.Source = streamMediaSource.CreateMediaPlaybackItem();
streamMediaPlayer.Play();

await Task.Delay(10 * 1000);

streamMediaSource = await FFmpegInteropX.FFmpegMediaSource.CreateFromUriAsync(url);
streamMediaPlayer.Source = streamMediaSource.CreateMediaPlaybackItem();
streamMediaPlayer.Play();

The second Method Play() will crash after a few seconds whatever the new url is.

Exception thrown at 0x00007FFAE8C71F70 (FFmpegInteropX.dll) in app.exe: 0xC0000005: Access violation writing location 0x0000000000000028.
brabebhin commented 2 years ago

You need to wait for the media opened event in order to call play. That task delay is likely causing some timimg issues.

lukasf commented 2 years ago

I think the problem ist that you do not keep reference to the first FFmpegMediaSource while the second one is being opened by MediaPlayer. MediaPlayer will keep playing the first source until you received either opened or failed for the second source. So you must keep reference to the first source during opening the second source. If you check our samples, you will see that we actually have two fields for FFmpegMediaSource. One is for the currently playing, and one is for the next one that is being opened. One field will be nulled out after Opened or Failed event from MediaPlayer. Only then we can safely release references to the previous file.

If you don't properly keep reference, you will run into access violations, which is what you see.

yikuo123 commented 2 years ago

@brabebhin @lukasf Thanks a lot!

I have tried what you said, but it still crashes sometimes. So, I switch to the original Microsoft project, it does not crash my app.

And then I try to rollback FFmepgInteropX to version 0.9.3 and FFmpegInteropX.FFmpegUWP to version 4.1.100.

It works perfect! No crashes! No uncatchable "Access violation" exceptions! It works as what I want it to!

Maybe it is a bug on the FFmpegInteropX or the FFmpeg project, but I'm not sure about it.

brabebhin commented 2 years ago

Please provide a complete code sample that reproduces the issue.

yikuo123 commented 2 years ago

Hello @brabebhin,

I uploaded 2 sample projects to reproduce the issue: The project App use FFmpegInteropX 1.0.1 + FFmpegInteropX.FFmpegUWP 5.1.100. The project App2 use FFmpegInteropX 0.9.4 + FFmpegInteropX.FFmpegUWP 5.1.100.

There are no any other differences between the 2 projects.

And there are two scenarios in each of the project: Scenario 1 shows playing valid media url and reset Source of MediaPlayer Scenario 2 shows playing invalid media url.

FFmpegInteropX 0.9.4 works well, but FFmpegInteropX 1.0.1 always throws uncaughtable exception in both of the 2 scenarios.

test2.zip

brabebhin commented 2 years ago

`public sealed partial class MainPage : Page { private MediaPlayer _mediaPlayer; private FFmpegMediaSource _lastMediaSource; private FFmpegMediaSource _currentMediaSource;

    public MainPage()
    {
        this.InitializeComponent();
        this.Loaded += MainPage_Loaded;
    }

    private async void MainPage_Loaded(object sender, RoutedEventArgs e)
    {
        _mediaPlayer = new MediaPlayer();
        _mediaPlayer.AutoPlay = true;
        _mediaPlayer.AudioCategory = MediaPlayerAudioCategory.Movie;
        _mediaPlayer.MediaOpened += MediaPlayer_MediaOpened; ;
        _mediaPlayer.MediaFailed += MediaPlayer_MediaFailed; ;
        MyMediaPlayerElement.SetMediaPlayer(_mediaPlayer);

        await Play();
    }

    private async void MediaPlayer_MediaOpened(MediaPlayer sender, object args)
    {
        await Task.Delay(3000);
        await Play();
    }

    private async void MediaPlayer_MediaFailed(MediaPlayer sender, MediaPlayerFailedEventArgs args)
    {
        await Task.Delay(3000);
        await Play();
    }

    private async Task Play()
    {
        await PlayValidMediaSource();
        //PlayInvalidMediaSource();
    }

    // Scenario 1, Playing valid media source, throws uncaughtable exception "Access violation"
    private async Task PlayValidMediaSource()
    {
        //_lastMediaSource?.Dispose(); // this also throws uncaughtable exception, ignore it now
        _lastMediaSource = _currentMediaSource;

        _currentMediaSource = await FFmpegMediaSource.CreateFromUriAsync(@"http://www.w3school.com.cn/example/html5/mov_bbb.mp4");
        _mediaPlayer.Source = _currentMediaSource.CreateMediaPlaybackItem();
    }

    // Scenario 2: Playing invalid media source, throws uncaughtable exception "Access violation"
    private async void PlayInvalidMediaSource()
    {
        try
        {
            var mediaSource = await FFmpegMediaSource.CreateFromUriAsync("https://example.com/");
            _mediaPlayer.Source = mediaSource.CreateMediaPlaybackItem();
        }
        catch (Exception e)
        {
            //FIXME: This code block can not catch "Access violation" exception!
            Debug.WriteLine($"Invalide media source, error={e}");
        }
    }
}`

I took the liberty to slightly modify your code. Can you try this version and see how it works out? Remember not to Dispose an active MediaSource.

yikuo123 commented 2 years ago

@brabebhin Thanks for your help!

I tried the modified code, it just works as before. After 3 or 4 times playing, it crashes my app by "Access violation writing location 0x0000000000000028."

lukasf commented 2 years ago

Hi @yikuo123,

thanks for providing the samples. Indeed I could reproduce the crashes here. It is a problem of accessing an AVIOContext field in the destructor without having a NULL check in there. This is already fixed on master branch, but I was not aware that this totally breaks all streaming/URL sources (since AVIOContext is never assigned in that scenario).

This is a pretty bad one, sorry for the inconvenience. We might have to release a 1.0.2 quickfix for this, because it will take a while until we create the next major release from master branch.

@brabebhin This makes me think about trying to re-activate unit tests once again. We would not have this bug, if our AppVeyor build would automatically execute at least a few basic tests. I have been thinking, can't we replace those damn unreliable UWP unit tests with a plain .NET Core unit test project? I mean, we don't really need any UI or app running, just to create our FFmpegMediaSource, right? We don't need UWP either (at least not our C++/WinRT version on master branch). I might give this a try in the next days.

brabebhin commented 2 years ago

I think our current master branch will fix the issue. I had trouble reproducing it cause the url would sporadically become unavailable. I couldn't confirm 1000% though yet.

As for unit tests, this is a tough one. I think we still need a full UWP environment for some APIs to work. I can give it a try tomorrow.

I think we should release the next version sooner, cause there was another issue that OP reported which was fixed with latest pre version.