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

Separate VideoPlayer and BlockingVideoPlayer game state classes #2348

Merged

Conversation

ivan-mogilko
Copy link
Contributor

@ivan-mogilko ivan-mogilko commented Mar 6, 2024

This is a limited backport from #2338, containing only improved VideoPlayer class, but no changes to script API, and no global working thread for non-blocking videos. Its main purpose is to keep code in branches at least somewhat consistent, but also make it easier to do improvements to video playback.

  • Separated VideoPlayer and BlockingVideoPlayer classes, where VideoPlayer is a generic player, while BlockingVideoPlayer defines a game state which runs VideoPlayer in particular way.
  • Turned BlockingVideoPlayer as another GameState implementation (complementary to the recent Implement "Game State" interface and several implementations #2332).
  • VideoPlayer now has buffer queue for video frames, supports both autoplay and manual frame-by-frame work modes.
  • No actual working thread is implemented in this PR, but it may be done later, to improve playing videos with higher framerates.

There's a potential issue with FLIC videos that I found. It looks like existing games that use FLIC has videos coded with 100 fps (!), but it's played at 60 fps as a effect from setting vsync in script, and that's how it matches the audio (which is played separately). This makes me concerned about maintaining compatibility with these games in case if a video will be run on a working thread on its true fps rate. I suppose this could be worked around by running the FLICs in manual mode using NextFrame() call from the main thread, rather than starting it with Play(). This is possible because FLICs don't have any audio in them.
EDIT: fixed this, but used an easier method.

@ivan-mogilko ivan-mogilko added context: code fixing/improving existing code: refactor, optimise, tidy... context: video (playback) labels Mar 6, 2024
@ivan-mogilko ivan-mogilko force-pushed the 362--blockingvideostate branch 2 times, most recently from cb08601 to 906f905 Compare March 7, 2024 12:34
@ivan-mogilko ivan-mogilko force-pushed the 362--blockingvideostate branch from 906f905 to befebbd Compare March 7, 2024 13:43
@ivan-mogilko ivan-mogilko force-pushed the 362--blockingvideostate branch from befebbd to 617743c Compare March 7, 2024 16:06
@ivan-mogilko ivan-mogilko force-pushed the 362--blockingvideostate branch 2 times, most recently from be218d5 to b78c1b1 Compare March 7, 2024 22:38
@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Mar 7, 2024

Ouf, I had to change few things in VideoPlayer compared to #2338, after finding some logical mistakes. But it seems working now.
I will need to update #2338 accordingly, maybe after merging this pr to master and ags4 branch, and rebasing 2338.

NOTE: I tested all video settings such as speed, looping etc by temporarily hardcoding them, because there's no script commands for these in ags v3.*.

@ivan-mogilko ivan-mogilko force-pushed the 362--blockingvideostate branch from b78c1b1 to 86628c8 Compare March 8, 2024 11:21
@ivan-mogilko ivan-mogilko merged commit 337b4ae into adventuregamestudio:master Mar 8, 2024
20 checks passed
@ivan-mogilko ivan-mogilko deleted the 362--blockingvideostate branch March 8, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context: code fixing/improving existing code: refactor, optimise, tidy... context: video (playback)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant