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

VideoCommon: FrameDump fixes/cleanups. #9182

Merged
merged 1 commit into from
Oct 23, 2020

Conversation

jordan-woyak
Copy link
Member

Fixed calculation of PTS values.
This should fix: https://bugs.dolphin-emu.org/issues/12228
Warnings are issued on dropped/skipped frames.

Filename of dumped video now includes the GameID and timestamp like our screenshots.
e.g. "G9SE8P_2020-10-22_11-51-30_0.avi"

Dumping previously restarted on resolution change.
It now also restarts on VI refresh-rate change and savestate load.
Note these restarts maintain the original timestamp to help with file organization/splicing.

Eliminated static variables and general cleanup in FrameDump.
Variable renames and minor cleanup of related code in RenderBase.

@lioncash lioncash merged commit 87e4a07 into dolphin-emu:master Oct 23, 2020
@ghost
Copy link

ghost commented Oct 24, 2020

@vadosnaprimer
@thecoreyburton
Make a test with this and tell us what do you think, it happened to get merged rather quick. However this is not the whole thing, but it's the kinds of stuff I had on my mind as well, namely the filename creation format.
I'll update the progress here #8121 as usual.

@vadosnaprimer
Copy link
Contributor

vadosnaprimer commented Oct 24, 2020

Maybe it fixes something, but I can't notice any improvements since #8347 (comment) and #8347 (comment) Due to this, I can't test if segmented AV desync is still there.

Filename logic looks good since it won't overwrite the previous dump now (and filename base remains the same across all segments), tho with audio it still will.

@ghost
Copy link

ghost commented Oct 24, 2020

Yeah, but it wasn't expected anyway, I was just wondering even tho TAS stuff more special, but this is nonetheless a step in the right direction, eventually, and it's long overdue, but I still have a couple of months of other stuff I have to finish, I'd be back at this right about now, if I wouldn't start a new PC build.

Although I don't exactly understand why you can't test? Surely you didn't figure it all out in less than an hour?
I didn't had a chance to actually figure out what the PTS code changes were, I'm going to take a deeper look later, but perhaps OP can explain.

@vadosnaprimer
Copy link
Contributor

vadosnaprimer commented Oct 24, 2020

I mean all the initially duplicate frames at frame 0 are missing from the dump, so there's still huge initial AV desync (most apparent with Resident Evil 4), which is why I can't check if it still desyncs further with every new segment dumped (it was desyncing with segmented dumping before frame 0 was broken).

@ghost
Copy link

ghost commented Oct 24, 2020

Yar, I wasn't as active/focused during those times when the frame duplication and AV desync PR was going around, I forgot some of it a bit too, so I can't speak specifically right now, just that this PR I think wasn't even meaning to fix any of that, it started as a basic code cleanup with some stuff added. I think you should discuss the details of that overall issue in the #8121 ticket.

@JosJuice
Copy link
Member

JosJuice commented Oct 24, 2020

@altimumdelta #8121 (or any other PR for that matter) is not appropriate to use as a centralized ticket for everything related to frame dumping. If you want something like that, the issue tracker would be much better place, or perhaps the forums. And even then, it is better to file a separate report on the issue tracker for each issue rather than trying to have some kind of frame dumping megaissue, so that it's possible to keep track of what has been fixed and what hasn't.

@ghost
Copy link

ghost commented Oct 24, 2020

@JosJuice Right about now is when I started thinking the same thing, you're right, although it would be quite a chore to fill out all these issues into the bug tracker one by one, there has to be like 30 of them, give or take, IMO. I do intend to update that PR with a proper branch eventually, just have to rebase and start over, however yes, I agree, it can't go on forever in terms of size and scope, so the main thing it's meant to be about is the update to the FFmpeg API calls.

Pokechu22 added a commit to Pokechu22/fifoci that referenced this pull request Nov 20, 2020
See dolphin-emu#26.  dolphin-emu/dolphin#9182 changed the default filename to be timestamped, instead of incremental; as such framedump0.avi is no longer created.

Only one file should be created, so the wildcard shouldn't cause issues.
delroth pushed a commit to dolphin-emu/fifoci that referenced this pull request Nov 20, 2020
See #26.  dolphin-emu/dolphin#9182 changed the default filename to be timestamped, instead of incremental; as such framedump0.avi is no longer created.

Only one file should be created, so the wildcard shouldn't cause issues.
@jordan-woyak jordan-woyak deleted the frame-dump-cleanup branch December 30, 2020 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants