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

Align frame advance and movies to full field boundaries #8347

Open
wants to merge 1 commit into
base: master
from

Conversation

@hosaka-corp
Copy link
Contributor

commented Sep 4, 2019

Currently, frame-stepping and Movie::FrameUpdate() are dealt with at the beginning of each "active video field" (see Core::Callback_VideoCopiedToXFB()).

This is [conceptually] inconsistent with accurate SI polling emulation (#8236 and other WIP branches) because SI polls may occur in the space between active video fields. Here's some log output from a game that does SI polling four times in one field:

// Current behavior
Core/HW/VideoInterface.cpp:809 I[SI]: new video interrupt, half_line=0000
Core/HW/VideoInterface.cpp:741 I[SI]: new field, half_line=0000 (start of full field)
Core/HW/VideoInterface.cpp:749 I[SI]: new poll, half_line=000f
VideoCommon/VideoBackendBase.cpp:73 I[SI]: Video_BeginField (start of active field)
!!!!!! Core/Core.cpp:843 I[SI]: <Movie::FrameUpdate() happens here>
VideoCommon/RenderBase.cpp:1336 I[SI]: Renderer::Swap() ended
Core/HW/VideoInterface.cpp:749 I[SI]: new poll, half_line=00ab
Core/HW/VideoInterface.cpp:749 I[SI]: new poll, half_line=0147
Core/HW/VideoInterface.cpp:749 I[SI]: new poll, half_line=01e3

// New behavior
Core/HW/VideoInterface.cpp:809 I[SI]: new video interrupt, half_line=0000
Core/HW/VideoInterface.cpp:741 I[SI]: new field, half_line=0000 (start of full field)
!!!!!! Core/Movie.cpp:208 I[SI]: <new Movie::FrameUpdate() happens here>
Core/HW/VideoInterface.cpp:749 I[SI]: new poll, half_line=000f
VideoCommon/VideoBackendBase.cpp:73 I[SI]: Video_BeginField called (start of active field)
Core/Core.cpp:843 I[SI]: Callback_VideoCopiedToXFB
VideoCommon/RenderBase.cpp:1336 I[SI]: Renderer::Swap() ended
Core/HW/VideoInterface.cpp:749 I[SI]: new poll, half_line=00ab
Core/HW/VideoInterface.cpp:749 I[SI]: new poll, half_line=0147
Core/HW/VideoInterface.cpp:749 I[SI]: new poll, half_line=01e3

This ensures that "a movie frame" (and the check for frame-stepping) starts at the same time as a "full field."


// Called at field boundaries in `VideoInterface::Update()`
bool FrameUpdate(void)

This comment has been minimized.

Copy link
@JosJuice

JosJuice Sep 4, 2019

Contributor

We typically don't put void in the parameter list.

Any particular reason why this returns a value?

This comment has been minimized.

Copy link
@hosaka-corp

hosaka-corp Sep 4, 2019

Author Contributor

Nope! Leftover from checking something.

@@ -733,6 +733,12 @@ static void EndField()
// Run when: When a frame is scanned (progressive/interlace)
void Update(u64 ticks)
{
// If this half-line at a field boundary, potentially deal with frame-stepping

This comment has been minimized.

Copy link
@JosJuice

JosJuice Sep 4, 2019

Contributor

I think there's a word missing here.

@hosaka-corp hosaka-corp force-pushed the hosaka-corp:frame-advance-alignment branch from 5549c6b to 0ea5463 Sep 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.