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

IPU: Implement start code validation for BDEC/IDEC #6270

Merged
merged 2 commits into from May 30, 2022
Merged

IPU: Implement start code validation for BDEC/IDEC #6270

merged 2 commits into from May 30, 2022

Conversation

Goatman13
Copy link
Contributor

Description of Changes

According to MPEG 1/2 specs when 8 "0" bits are found, we should check for 000001xxh. Recommended way is to discard bits until stream is aligned. Then check 24 bits for 000001h, and if they won't match discard 8 bits and try again until valid code is found, or stream ends.

Rationale behind Changes

Fixes #1141
Also helps Tiger Woods 06/08 from #2132
WRC3 patch removed as a bonus, game work also on master.

Suggested Testing Steps

Test that other games aren't broken while playing movies. Test that movies aren't corrupted, don't skip, etc.
I tested many random games i own this time, and all of then worked. I wasn't able to test "billboards" in Test Drive game, and i suspect that PR can help it. Games which still have IPU entries in GameDB are still broken, no need to test them really.

Thx to @raul3d for pointing out Play! commit which fixed Onimusha, that helped a lot.
This implementation is little bit different from what Play! do, I extended code search to BDEC (more precisely to mpeg2_slice). We also don't accept code if there is no data after first 8 bits after alignment. This break many games in non intra mode. But if we return false instead, and wait for data, everything seems to work ok. This was needed for Tiger Woods PGA games.

According to MPEG specs when 8 "0" bits are found, we should check for 000001xxh. Recommended way is to check 24 bits (000001h), and if they won't match discard 8 bits and try again until valid code is found, or stream ends.
@refractionpcsx2
Copy link
Member

Okay I went through games which have skipmpeg, plus one that I know has a patch, and these were my results

Penny Racers videos more reliable, still needs patches to avoid jump to null.
David Beckham Soccer still hangs (But does actually work with mild EE underclock, so likely timing related and not a result of this PR.)
ESPN NBA 2K5 still broken (Still requires SkipMPEG else the end of the ESPN video hangs)
Tiger Woods 06 Fixed
Tiger Woods 08 Fixed
Onimusha Dawn of Dreams Fixed (as promised)
Pro Mahjong Kiwame Next (Version 1.00) still broken, seems to loop reading on a CD sector (could be a bad old CDVD driver at fault) so still needs patch. 3.01 works fine (on master too)

But that's 3 (and I guess a fourth too) fixed, I didn't find anything broken. nice work! :)

@refractionpcsx2 refractionpcsx2 added this to the Release 2.0 milestone May 29, 2022
@seta-san
Copy link
Contributor

N NBA 2K5 still broken (Still requires SkipMPEG else the end of the ESPN video hangs)

the 2k games are more likely dev9 issues

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented May 30, 2022

@seta-san This is unrelated, the DEV9 issue is patched.

@refractionpcsx2 refractionpcsx2 merged commit c8e0090 into PCSX2:master May 30, 2022
@guidodi
Copy link

guidodi commented Jun 2, 2022

This broke videos on Tales of Rebirth. I cant send log or gsdump right now, wont be in my house for some time but I thought I would mention it anyway.

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Jun 2, 2022

This broke videos on Tales of Rebirth. I cant send log or gsdump right now, wont be in my house for some time but I thought I would mention it anyway.

a GSDump is no good, it's a core problem. I have actually already noted it over at #2132

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Jun 2, 2022

Looks like it's fixable with gamefixes (EE Timing hack, the videos seem to hang later though, checking cycle rate changes)

@refractionpcsx2
Copy link
Member

Okay so this PR does cause the videos to hang at the end of the video, the very last frame stops and it loops the last 5-10 frames, but the corruption can be solved with the EE Timing hack.

I'm not sure if this is a DMA problem that's readed its ugly head or if it's just forever checking for a start code and failing or something, idk

@refractionpcsx2
Copy link
Member

Looks like this PR basically showed up DMA timing errors, so I have redone them somewhat in #6313 and it seems to resolve both the glitching and the hangs.

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Jun 3, 2022

hey, sorry, got another one for you @Goatman13 which broke with the addition of your PR.

Onimusha 3 Demon Siege dies right at the beginning during the lang/refresh selection, wonder if you could look in to it :)
Onimusha 3 [SLES 51914] (E) (2022-05-03 21-26-16).zip

if you get as far as the refresh rate selector, you've probably fixed it, but to be sure, pick 50hz and a video should start playing.

@Goatman13
Copy link
Contributor Author

Game never send next start code. It seems that i need to modify that loop little bit. I will PR tomorrow, I already got it working. I missed part where game can send something else than 0 or 1. Looks like in that case we shouldn't set SCD, but at the same time we should stop looking for code, and fall back to next case.

@refractionpcsx2
Copy link
Member

isn't 1 the only valid thing for start code detection though?

@Goatman13
Copy link
Contributor Author

Goatman13 commented Jun 3, 2022

Yes it is. But at the same time i found different source of MPEG2 docs, and there they mention removing only 0 bits while searching for code. Previous source i had mentioned only removing stuffing bits.

So it looks like when there is something else than 0 or 1 in 24 bit search, it is invalid. But there is no description of that situation in docs. Onimusha send 0x200 (24 bit)... I figured that abandoning that search without setting ctrl SCD work for it. But maybe only because that basically set value in bp top, and end command.

While current code made game to gave up on IDEC due to timeout.

@refractionpcsx2
Copy link
Member

yeah the manual I have (which I should really read lol) says this

image

but it doesn't mention checking for 8bits of 0 beforehand like we do, idk where that comes from, or why we do it unaligned, it's all very confusing to me

@Goatman13
Copy link
Contributor Author

Goatman13 commented Jun 3, 2022

Actually that manual mention removing zero bits first, that why we do it until stream is aligned. Then remove zero bytes if stream is aligned.

Edit: keep in mind that variable bit8 in pcsx2 is just name. When you do getBits8 on unaligned stream, it will return only bits until align, and zero for upper part.

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Jun 3, 2022

I see.... I'm a little confused to what you mean, cos we don't remove zero bits until we're aligned, we read 8 bits, presumably to check for stuffing, and if it's 0 stuffed, then we align the stream, THEN check for start code.

also is it not true that the 8 bits we check might not always be zero if the stream is misaligned?

@Goatman13
Copy link
Contributor Author

Goatman13 commented Jun 3, 2022

Read my edit in post above just in case.

"I'm a little confused to what you mean, cos we don't remove zero bits until we're aligned, we read 8 bits, presumably to check for stuffing, and if it's 0 stuffed, then we align the stream, THEN check for start code"

g_BP.Align = g_BP.Advance to align. So removing 0 bits until we align stream.

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Jun 3, 2022

well, yeah, it adds 7 then chops it to the next 8bit aligned position down from there, it doesn't quite remove 0's until it's aligned, more skips to an aligned state xD

@Goatman13
Copy link
Contributor Author

That what DUMPBITS do under the hood. Whole decoder dump bits that way, so i assumed that how it's done on IPU. I don't think that other stuff will work if that current "skipping" bits is not what should be done.

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Jun 3, 2022

Oh I never said it was wrong :P I guess my biggest issue was I misunderstood what getbits8 was doing when misaligned.

@Goatman13
Copy link
Contributor Author

Biggest issue is that none of available documents explain what to do when something else than 0 or 1 is hit in 24 bit search.

@refractionpcsx2
Copy link
Member

Well the one I have above seems to just imply that keep going until it is 1, or is it interpreted as an error code? cos surely the only 2 things it should be expecting is

  1. zero stuffing
  2. a start code

Am I wrong there? :/

@Goatman13
Copy link
Contributor Author

Going until is 1 is what break Onimusha 3. Because 1 is never found.

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Jun 3, 2022

I did just try changing it to say if it's greater than 1, set ECD and break, and that worked for Onimusha yeah

That fixes Sims Bustin Out too

@Goatman13
Copy link
Contributor Author

Yeah, i actually did that. But without ECD, and it worked too.
I need to test tomorrow, specially with other games.

@refractionpcsx2
Copy link
Member

Good luck :) It'd be nice to have some sort of solution (hopefully helping other games too)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Graphical Corruption on opening screens and title screen in Onimusha - Dawn of Dreams
4 participants