-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Video Capture: Split on resolution change and allow recording on 0x0 resolution. #13245
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
base: master
Are you sure you want to change the base?
Conversation
| Console.Error("GS: Unknown video mode. Please report: https://github.com/PCSX2/pcsx2/issues"); | ||
| return 0; | ||
| return 60; | ||
| //Console.Error("GS: Unknown video mode. Please report: https://github.com/PCSX2/pcsx2/issues"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may i ask why this was removed ? , also why are you defaulting to 60
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Video capture didn't start if fps was 0 due to assert(fps != 0). Like I said, code works but needs refactoring. Defaulted to 60 because then video capture would be using it. There most likely is better ways to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens is you want to record a PAL game ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It records at 50 as it should. Don't see how pal would be affected by this change. Unless on PAL Progressive which isn't supported by PCSX2 afaik and only used accessible by homebrew also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I'm wondering why does GetTvRefreshRate() even exist when there's VMManager::GetFrameRate() which calls GetVerticalFrequency() which gives the fps like GetTvRefreshRate would. I'm thinking that GetTvRefreshRate() call should get replaced by GetVerticalFrequency() or VMManager::GetFrameRate() call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also to my understanding this would have given 0 fps to video capture which would cancel it if 720p or 1080i was used. Which wouldn't happen now with my change to it but GetVerticalFrequency() would also handle this correctly.
This should be drafted then. |
Description of Changes
Add video splitting for every resolution change. Allow recording from frame 0 (if using "Pause on Start") or when resolution is 0x0 / No Image, this will create blank video with 640x480 resolution. Before on blank frames it used last visible frame that was visible but now it's blank.
Rationale behind Changes
Instead of stretching video we can keep the resolution as is and just create a new file for each resolution, and later stretch or do something else with raw footage. Fixes not being able to start video capture when starting emulation. Fixes blanks using last visible frame.
Suggested Testing Steps
Test recording from frame 0, when resolution changes, during 0x0 resolution (should result in a 640x480 video).
Did you use AI to help find, test, or implement this issue or feature?
Yes, asked copilot to explain why blank frame used last visible frame in video capture.
This works but in my opinion code needs refactoring.