deps/media-playback: Fix potential crash when doing swscale in media.c

[hellowanda]

Description

There is a potential crash when doing swscale in media.c: If frames decoded from a media file have different pixel format/width/height..(such as a video file which is remuxed from a series pictures with different format), it will always crash inside swscaler because we have no logic to recreate/update the swscaler now.

Motivation and Context

We should check swscale with every frames to make sure the swscaler works correctly.

How Has This Been Tested?

Tested on 64bit Windows 10

Types of changes

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• [x] My code has been run through clang-format.
• [x] I have read the contributing document.
• [x] My code is not on the master branch.
• [x] The code has been tested.
• [x] All commit messages are properly formatted and commits squashed where appropriate.
• [x] I have included updates to all appropriate documentation.

[WizardCM]

Hi there @hellowanda - thank you for submitting this bug fix. Would you be able to provide a test file for us to verify the original bug and the new behaviour with this bug fix? Thanks!

[hellowanda]

Hi there @hellowanda - thank you for submitting this bug fix. Would you be able to provide a test file for us to verify the original bug and the new behaviour with this bug fix? Thanks!

Oh, I'm very sorry, I just noticed your comment . This is the test file : test-file.zip

[PatTheMav]

@derrod what's your take on this? If I understand it correctly, this intends to fix the issue of a media file having different formats per frame so a single swscale instance for the entire file could potentially crash if it gets a sudden "bad" frame.

Instead it checks the format for every new frame and creates a new scaler for the incoming frame if needed.

[derrod]

A video file could be comprised of several segments with different formats and still be legal/spec conformant, so this is probably valid. But with all the changes made to media-playback over the last two years I don't know if this issue still exists/would need to be solved differently (the PR is conflicted as well),

@tt2468 seems to have intended to test this PR but didn't get around to it yet, maybe he has some more recent experience with these problems.

[tt2468]

The why: Indeed many formats either officially or unofficially support adaptive resolution, allowing a switch from say 1080p to 720p mid-playback. In programs like VLC and FFplay, this is generally handled explicitly and is supported. As such, it makes sense for us to support it, especially if playback of livestreams is to be supported (example: Streamlabs Mobile).

This is technically the only thing required to fully support adaptive resolution playback on our end. The issue still exists. It's uncommon for the swsscale codepath to be gone down, but of course it will lead to a crash if it's being used and any codec parameters change.

One thing I should note is that a number of hardware accels in ffmpeg have broken codec param change detection logic, resulting in a crash regardless of if our side "supports" it or not. (the decoders do not detect the changes, and crash inside of ffmpeg). The most notable one is nvdec. x264 decoding works great though.

[PatTheMav]

@tt2468 I happily defer to you if this change is still good as-is (apart from a rebase).

[PatTheMav]

@hellowanda Please rebase your changes onto current master branch.

@tt2468 Do you have any further objections to merging this? If I understood you correctly this might still lead to a crash when hardware-accelerated decoding is used?

[tt2468]

The crash with hardware decoding is an unrelated issue actually. It's an issue with ffmpeg, not an issue that this PR is intended to solve. I agree that this needs to be fixed, but I'm not sure I like the methodology used here. Give me up to 5 days to suggest an alternative method. If I don't get around to it, then this PR is fine to be merged as long as it's tested against the current version of media-playback.

[PatTheMav]

@hellowanda please rebase your PR upon current master. There are conflicts that need to be resolved before we can merge this, otherwise we'll have to close this PR due to inactivity.