Skip to content
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

AGS: Avoid playing unsupported WMV videos and look for alternatives #5381

Merged
merged 1 commit into from Nov 5, 2023

Conversation

tag2015
Copy link
Contributor

@tag2015 tag2015 commented Oct 22, 2023

A few (old) AGS games use WMV videos, which are not supported.
Notable examples are:
The Marionette https://archive.org/details/the-marionette
EGRESS https://kramsdesign.com/egress.html
Ghostdream (Steam)
However, the files can be easily converted to supported formats using ffmpeg or similar software.

This PR simply looks for files with the same name but a supported extension and plays those in place of the original,
allowing players to add their replacements.
Also shows a warning that wmv files are not supported (and avoids crashing the entire engine)
The warning says "Consult ScummVM wiki for details", I plan to add a brief explanation on the AGS engine page.

Prevent playing unsupported WIndows Media videos to avoid engine crashing,
and look for alternative formats. Also warn the user if no alternative is available
and continue.
@tag2015 tag2015 requested a review from criezy October 22, 2023 17:58
@bluegr
Copy link
Member

bluegr commented Oct 29, 2023

The part where replacement videos are checked in place of the unsupported WMV ones makes sense. However, why did you extend this to other video formats, as well (e.g. MPGs and OGVs)? It doesn't make sense for players to transcode game videos, if they are properly supported.

@tag2015
Copy link
Contributor Author

tag2015 commented Oct 29, 2023

The part where replacement videos are checked in place of the unsupported WMV ones makes sense. However, why did you extend this to other video formats, as well (e.g. MPGs and OGVs)? It doesn't make sense for players to transcode game videos, if they are properly supported.

No, it's not supposed to do that. Maybe the code is not very clear?
It works the exact same way as before, looking at the extension and calling the relative function (play_theora or play_mpeg etc.). The only difference is that it uses a pointer (file_ext) to the extension inside filename instead of doing strlen(name) - 3. If file_ext is NOT wmv/wfl it just calls the relative function, otherwise it replaces it with the first candidate (ogv) and sets the wmv boolean. If an ogv file is not available, it replaces the extension with "mpg" and so on. If none is found, a warning message is shown.

By the way if you want to test the PR, Tales from the Outer Zone 1 uses an OGV movie at the beginning and should play normally. EGRESS uses a WMV file and should display the warning

@bluegr
Copy link
Member

bluegr commented Oct 30, 2023

The part where replacement videos are checked in place of the unsupported WMV ones makes sense. However, why did you extend this to other video formats, as well (e.g. MPGs and OGVs)? It doesn't make sense for players to transcode game videos, if they are properly supported.

No, it's not supposed to do that. Maybe the code is not very clear? It works the exact same way as before, looking at the extension and calling the relative function (play_theora or play_mpeg etc.). The only difference is that it uses a pointer (file_ext) to the extension inside filename instead of doing strlen(name) - 3. If file_ext is NOT wmv/wfl it just calls the relative function, otherwise it replaces it with the first candidate (ogv) and sets the wmv boolean. If an ogv file is not available, it replaces the extension with "mpg" and so on. If none is found, a warning message is shown.

By the way if you want to test the PR, Tales from the Outer Zone 1 uses an OGV movie at the beginning and should play normally. EGRESS uses a WMV file and should display the warning

Sorry! My bad. Should have checked the code better, especially the && wmv part :)

if (!play_success && wmv) {
    ....
}

@sev-
Copy link
Member

sev- commented Nov 5, 2023

Thanks, merging.

I wonder, which codecs are we missing here. Perhaps, those could be added relatively easily?

@sev- sev- merged commit c2bc9a2 into scummvm:master Nov 5, 2023
8 checks passed
@tag2015 tag2015 deleted the ags_videofix branch November 10, 2023 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants