rsp hle cleaning

[Bobby Smiles]

Hi,

I've seen that ecsv has started to clean the hle plugin source [1] and I
welcome this change very much !

Few month back I've attempted to clean some bits as well [2] but wasn't
fully satisfied with the result (messy commits, no strong non regression
guaranty, ...) so I didn't asked it to be merged. But since some work is
going in this direction, I thought I could share publicly what I've
tried, as it might help others.

My work included among other changes :
- better distinction between each version of ABI
- less code duplication
- implement POLEF audio command (which fixes the intro in Pilotwing)
- implement SEGMENT audio command (which no games seems to use)
- implement RESAMPLE_ZOH audio command (which fixes weird sound when
switching to headset mode in 1080 snowboarding)
- Fix left/right channel swap in ucode1

Feel free to pick any bit of code that improve the code quality, and ask
questions as necessary !


[1] https://github.com/ecsv/mupen64plus-rsp-hle/tree/nocpp
[2] https://bitbucket.org/bsmiles32/mupen64plus-rsp-hle_audio/

[Dorian Fevrier]

I can't really help but I love your list. :)

Thanks for contribute. :)

Regards,

Dorian

Le 2014-01-01 09:10, Bobby Smiles a �crit :

[Sven Eckelmann]

On Wednesday 01 January 2014 15:10:11 Bobby Smiles wrote:
> Hi,
>
> I've seen that ecsv has started to clean the hle plugin source [1] and I
> welcome this change very much !
>
> Few month back I've attempted to clean some bits as well [2] but wasn't
> fully satisfied with the result (messy commits, no strong non regression
> guaranty, ...) so I didn't asked it to be merged. But since some work is
> going in this direction, I thought I could share publicly what I've
> tried, as it might help others.
>
> My work included among other changes :
> - better distinction between each version of ABI
> - less code duplication
> - implement POLEF audio command (which fixes the intro in Pilotwing)
> - implement SEGMENT audio command (which no games seems to use)
> - implement RESAMPLE_ZOH audio command (which fixes weird sound when
> switching to headset mode in 1080 snowboarding)
> - Fix left/right channel swap in ucode1

[02:22:14] <Richard42> ecsv, I see that you have a separate branch of the rsp
which has a lot of modifications from bobby smiles
[02:22:26] <Richard42> do you have any thoughts about how or when to merge all
of this work to the master branch?
[...]
[08:21:55] <ecsv_> Richard42: i started to check these patches twice and came
to the conclusion that it is too messy and conflicts everywhere
[08:22:09] <ecsv_> not only with my patches but also with his own patches
[08:22:40] <ecsv_> it basically touches ~70% of all lines
[08:23:55] <ecsv_> these are the patches mentioned by him in "rsp hle
cleaning"
[08:24:18] <ecsv_> i think he wrote that mail to the mailing list on the
2014-01-01
[08:25:16] <ecsv_> even when I basically aggree with what he did in the
branch, it is currently in a state which is not mergable (just by the amount
of conflicts and non-clean commits)
[08:28:50] <ecsv_> I thought about redoing all the changes by him but gave up
after checking the changes of the rest of the ~180 commits. in the end i would
just add more bugs in this process (which would be totally my fault) than
getting things fixed by his changes
[08:29:20] <ecsv_> I just hadn't the guts to write it to him
[08:30:31] <ecsv_> btw. i saved some work of other people which didn't submit
them to some branches in the mupen64plus-repos during the git conversion

Any suggestion how we should proceed? I personally stopped to work on rsp-hle
after seeing your big pile of changes.

Kind regards,
Sven

PS: Just for the future (this is important for ports like mupen64plus-ae,
libretro-mupen64plus, ...): Please submit small chunks of changes and don't
wait too long to do it. It is just a lot easier for someone to merge it when
it is small just because it is a lot easier to read and modify it when
something goes wrong.

[bobby.s...@gmail.com]

Sorry for this awkward situation. My work is indeed unmergeable.
I just published it to allow people to see what I had in mind for the refactoring, not for directly pushing it.

If you agree, I'll rework all these changes and submit them in a more logical and incremental form, now that I have more experience with the audio ucode
(I reverse engineered almost all ~20 revisions of them + the musyx ucode).

Still, I'd be interested in some feedback on my work (if we leave apart the messy commit history...) in order to improve its quality (code readability, file organization, standard compliance, ...).
I'll give you some more background on the audio-list based ucodes in order to help you review the code :

I classified audio-list based ucode in 3 "families" (audio, naudio, NEAD), each composed of several revisions/versions.

-audio is probably the first and certainly is the most used version: almost all 3rd parties dev used that.
It was almost well reverse engineered, and there were 2 audio commands not emulated (SEGMENT and POLEF).
There were 2 other revisions (both developped by Rare) used by GoldeneEye and BlastCorp.

-naudio is a latter version which simplified some aspect of the audio ucode (envmixer is linear instead of exponential, buffers are fixed, ....)
It was also distributed to third parties and the secondly most used ucode.
There were 4 other revisions (all developped by Rare) that added for instance MP3 decoding.

-NEAD (for Nintendo Entertainment Analysis & Development) seems to be used by Nintendo dev only.
There are many versions/revisions, almost each of them made for a particular game.
The order in which they were developped, or who inherit from who is not very clear.
Their ABI can differ in incompatible ways which caused for instance the isMK/isZelda hack, and some commands marked UNKNOWN.

The main difficulty during this refactoring (apart from finding descriptive names) is how to manage all these revisions/versions while keeping it maintainable.
I want to avoid as much code duplication as possible, but still convey all particularities/uniqueness of each ucode revision/revision.
If you have any suggestions toward that I'd welcome them very much.

Again I'm very sorry for the frustration caused to the team, and I hope that we can keep looking forward !
By the way, I created a github account so I can more directly contribute to the project :) and made my first pull request there as well !

Regards,
Bobby

[Sven Eckelmann]

On Friday 17 January 2014 02:40:18 bobby.s...@gmail.com wrote:
> Sorry for this awkward situation. My work is indeed unmergeable.
> I just published it to allow people to see what I had in mind for the
> refactoring, not for directly pushing it.
>
> If you agree, I'll rework all these changes and submit them in a more
> logical and incremental form, now that I have more experience with the
> audio ucode
> (I reverse engineered almost all ~20 revisions of them + the musyx ucode).

I would love it :)

As said before, I liked the general direction. It would be really helpful if
your explanation of these three categories could be found in the commit were
you changed the audio ucode files.

The structure at the end looked quite good to me but maybe someone with more
expertise on this subject has some suggestions.

[...]

> Again I'm very sorry for the frustration caused to the team, and I hope
> that we can keep looking forward !
> By the way, I created a github account so I can more directly contribute to
> the project :) and made my first pull request there as well !

Saw it. Congrats for your first successful github pull request :)

Kind regards,
Sven

[Richard Goedeken]

Bobby, thanks for all of your work on the RSP.  I can see that this is very good work and will be a big improvement to Mupen64Plus when complete.  Good luck with the refactoring; I'll can review the commits and send you comments on the quality/readability, and I'm sure Sven will as well.

Richard

--
You received this message because you are subscribed to the Google Groups "mupen64plus" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mupen64plus...@googlegroups.com.
To post to this group, send email to mupen...@googlegroups.com.
Visit this group at http://groups.google.com/group/mupen64plus.
For more options, visit https://groups.google.com/groups/opt_out.

[bobby.s...@gmail.com]

Okay pull request sent with a first pass cleaning [1] !
I didn't exactly reproduced what I've done in my previous work, but it should be even better :)
I'm quite happy with the result both code and commits :)

Next things to tackle are :
-envmixers audio commands refactoring
-ucode2 specific commands refactoring
-ucode3mp3 refactoring
-implement all version/revision subtleties
-implement missing audio commands (SEGMENT, POLEF, ...)

[1] https://github.com/mupen64plus/mupen64plus-rsp-hle/pull/7

[Richard Goedeken]

Hello N64 fans,

About a month ago I got an email from an emulator fan who is making a somewhat
radical suggestion, that we adopt the libretro port of Mupen64Plus as our main
source code trunk and make future releases based off of this, with the
RetroArch front-end. I'll copy his email here so you can read his arguments,
and I would like to hear your opinions on this.

Thanks,
Richard
-----------------------------------------------------------------------------

I see RA devs are making improvements to the codebase of the libretro port of
M64+. However, are these changes being ported back into the original M64+? If
not they really should. It would suck to see the two codebases diverge too much.

I have a novel solution to this. What if the Libretro version was made the
"main" version, and it was packaged with the RA frontend? It would gain:

1. A GUI
2. Superior synching, through Dynamic rate control
3. Netplay, although 2 player only
4. Shader support
5. The additional consoles that Libretro has been ported to

And since everyone would be working on the same version, all the changes are
kept in one codebase instead of having them diverge. The only thing is that
the M64+ devs would have to learn the Libretro API, but I don't think it would
be that hard.

For the end user, I don't think it would be a huge difference. They would run
the .exe, and there would be all the options, like load game, etc. It would be
packaged with only the M64+ libretro core. It would be easier than the current
GUI-less arrangement. I can't tell you how many times I've seen people
complain about the program and say "it does nothing" after double clicking it.

[Nathanael Ries]

Seems like a good plan to me.

[Sven Eckelmann]

On Sunday 19 January 2014 09:21:20 Richard Goedeken wrote:
> I see RA devs are making improvements to the codebase of the libretro port
> of M64+. However, are these changes being ported back into the original
> M64+? If not they really should. It would suck to see the two codebases
> diverge too much.

Unfortunately, we cannot actively search all the time to find possible forks
and sync it. So I wasn't aware of this fork/port for a long time.

This is how it is done in many other projects: Do your changes (even in a
quick and dirty way), clean your changes up, send them to upstream, discuss at
little bit (minimal work with Richard as maintainer) and get it merged. And at
the end everyone is happy (even third party people like mupen64plus-ae,
mupen64plus-pb, mupen64plus-...).

What usually ends bad for at least one of the involved parties: Fork it, do
some quick changes everywhere in the codebase, start to track your changes a
lot later, merge in some things which break it for other people - get some
other people to ask upstream to just drop their stuff and use the "superior"
fork from now on.

I was involved in getting such a problematic state resolved in mupen64plus-ae
in the past and luckily people like littleguy77 also started to fix these
things and send stuff upstream. But it looks more and more like that this in a
problem which happens again and again and again. Sorry, but you have to
understand that the extra thingy called libretro which just runs away alone in
a completely different direction without a sense of how to get everyone one in
the same boat (or at least in boats which wont break each other in the near
future) just gets me frustrated even more.

> I have a novel solution to this. What if the Libretro version was made the
> "main" version, and it was packaged with the RA frontend? It would gain:
>
> 1. A GUI

There are already GUIs/frontends for mupen64plus. The GUI stuff was removed
from the stuff maintained by Richard (now a library) to make it a lot easier
for ports to do their own stuff. Compare for example mupen64plus-ae with
m64py. Both are for completely different target platforms and still are doing
quite well

> 2. Superior synching, through Dynamic rate control

Can you tell us more about it? How is it integrated intro mupen64plus-core,
mupen64plus-video-* and mupen64plus-audio-*?

> 3. Netplay, although 2 player only

Can you tell us more about how they implemented it in the m64p core?

> 4. Shader support

Don't know what you mean here. The plugins are also using shaders. Your
comment sounds like "it has support to support something that is so widely
defined that it could mean that it supports anything"

> 5. The additional consoles that Libretro has been ported to

Are you sure that libretro has the magical powers to fix any architecture
problem just because it gets used? I doubt that.

And just doing a quick check in the source code already showed that the
porters are quite "good" in dropping stuff (instead of using the normal way of
disabling things during the builds) and doing quick hacks. Not sure if this
would please all people. Especially the removal of support for some savestate
types and similar things.

> And since everyone would be working on the same version, all the changes are
> kept in one codebase instead of having them diverge. The only thing is that
> the M64+ devs would have to learn the Libretro API, but I don't think it
> would be that hard.

It didn't seem like a lot of cooperation happened in the past together with
the libretro people. Did they actively tried to get to work with us and to
minimize the work for them and for us? I don't know why this would be changed
when we drop our stuff and start to use the libretro banner.

> For the end user, I don't think it would be a huge difference. They would
> run the .exe, and there would be all the options, like load game, etc. It
> would be packaged with only the M64+ libretro core. It would be easier than
> the current GUI-less arrangement. I can't tell you how many times I've seen
> people complain about the program and say "it does nothing" after double
> clicking it.

I can't say how many times I've seen people... wait, now we already start with
proofs based on anecdotes?

The mupen64plus-core is a library so it can be used quite easily in different
setups. There is an example/reference/... frontend called mupen64plus-ui-
console which gives basic access to everything necessary to get something
started. There are multiple frontend and launchers available which try to do
solve the problem of their specific platform.

Kind regards,
Sven

[Dorian Fevrier]

I'm not an active m64p member but as you ask for opinions:

- Yes, RA/Libretro rocks, it's a very good idea to provide a kind of
common API for every emulator.
- m64p need an GUI on x86 platform.
- From what I see (I hope I'm wrong on this), RA/Libretro devs don't try
to communicate with other devs from actively developed emulators (did
anyone here has been contacted by RA/Libretro devs?). I don't see this
as a good start for future. I have the impression the point is to
provide something cool faster but their doesn't seems to have a good
vision about how forked emulators could evolve later. They seem to "fork
and don't care about the rest".

My humble conclusion (I don't know Libretro):
- See how Libretro work, what limitations it imply (technically and from
a human side. If you want a lib that gather emulators, you need to be an
open mind I guess). As m64p provide an API. Maybe we can make libretro
comunicate to m64p via the API. This could be the best solution as you
can create a kind of external "libretro2m64p.dll" gate so libRetro use
libretro2m64p.dll which use mupen64plusXXXX.dll. This will still make
m64p separate (and so don't break potential platforms m64p can run on).
I guess few improvement to the current m64p API will be needed.

> What if the Libretro version was made the "main" version, and it was
packaged with the RA frontend?

To answer this: "Is there any risk to loose some platform m64p support
if we do so?" (Richard has make a huge work to keep m64p system agnostic).

Maybe discussions with the Libretro devs could be a good start to see
what they think about m64p (as they seems to have worked with the code.

Hope this help.

Regards,

Dorian

[libr...@gmail.com]

> RA/Libretro devs don't try
to communicate with other devs from actively developed emulators (did
anyone here has been contacted by RA/Libretro devs?)

This is not necessarily true.

The ones that I was successful with in getting the libretro port pushed
upstream were:

* SNES9x
* VBA-M
* Genesis Plus GX
* FBA
* Nestopia Undead
* FCEUmm
* PCSX ReARMed (fork by notaz)
* Picodrive
* bsnes (to a degree - Maister/Screwtape have a separate repo that is more or less synced with upstream)

I specifically went out of my way to get patches pushed upstream - even in instances where it was not especially easy to do so.

What might have changed is that I no longer actively go out of my way to do this again because some people (like clobber) started soliciting people from other
emulators to adopt libretro and it just got a bad response and I regret how this went down - and I wanted to avoid that in the future. I thought that it was better to
avoid that the next time - to basically 'build it and they will come' and once the port is in good shape, offer it to them and ask if they want it pushed upstream.

With regards to the Mupen64 libretro port -

> ""Is there any risk to loose some platform m64p support
if we do so?"

No I don't think so. If anything, it should be the opposite.

> And just doing a quick check in the source code already showed that the
porters are quite "good" in dropping stuff (instead of using the normal way of
disabling things during the builds) and doing quick hacks. Not sure if this
would please all people. Especially the removal of support for some savestate
types and similar things.

Well, we need savestate serialization for our purposes and file I/O-based savestates just doesn't agree with libretro's way of handling saves. You can notice in the Mupen64 libretro that saving doesn't create any performance hiccups due to the stream-based nature. We haven't been able to get 'rewind' working yet though so we could definitely do with some collaboration with the Mupen64 core devs there on how to reshape this so that we could do that with savestates.

I am  totally willing to go back and come up with a cleaner implementation without removing things that are in the existing codebase - as long as we are able to do savestates and save RAM without it having to be file I/O-based.

> Sorry, but you have to
understand that the extra thingy called libretro which just runs away alone in
a completely different direction without a sense of how to get everyone one in
the same boat (or at least in boats which wont break each other in the near
future) just gets me frustrated even more.

I am totally willing to do a rebase of the current Mupen64 libretro port that plays better with upstream if that is what you feel is needed. It's just that the libretro port requires things to be done differently as opposed to how you guys do it - for instance, the entire libretro port (plus plugins) gets baked into one dynamic library (instead of several separate ones in the case of Mupen64) and with that we have to keep in mind that certain structures might have the same name definitions - and therefore we have to make subtle alterations to their names to get it to link at the end (such as the 'rsp' structure in RSP HLE). So it isn't a case where I just did a fork for the heck of it and just to spite you guys - it is for the reason that the libretro port is not a straightforward port like the rest and I didn't want to be demanding to you guys and start requesting all sorts of things on your behalf which might drive people away even more.

> Don't know what you mean here. The plugins are also using shaders. Your
comment sounds like "it has support to support something that is so widely
defined that it could mean that it supports anything"

He means user-defined shaders. Think of them as additional post-processing shaders not tied to Glide64/glN64/Rice/whatever plugin you can think of.

[paulsc...@gmail.com]

Thought I'd put in my two-cents worth on this.

My guess is that Mupen64Plus was chosen as the basis for the libretro port because of its portability and system-independance, not in spite of those qualities.  Richard and his team have done a commendable job in removing everything platform-specific and making Mupen64Plus a portable library.  In the short term, locking the emulator back into a specific framework probably would have an immediate boost in visibility and numbers of systems Mupen64Plus can run on.  But on the flip side of that, in the long term when the next big idea comes along (I won't speculate on what that idea might be), Mupen64Plus will no longer be as obvious a choice (a fork desynchronized from upstream prior to the libretro switch would almost certainly be more appealing in that case).

So short-term, switching to libretro would probably would have a lot of benefits, but I think long-term the decision would lead to even further fragmentation and eventual stagnation of the original project.  I would certainly prefer to see non-libretro-specific improvements and bugfixes cleaned up and send to upstream for discussion and merged at Richard's discretion (as Sven pointed out, I've not been very good about this myself in the past, so I'm probably not one to talk..)  Either way, I just want to say that I'll support Richard's decision.

[littleguy77]

Any way to split topics in this forum?

Since you asked:

- I think the original "libretro fan" has vastly oversimplified things.

- I share Sven's concerns about fragmentation.

- I'm glad to hear Libretro's interest in pushing back upstream.

But TBH I think the only people's opinions that matter on this issue are Richard, Sven, and Libretro.

On a related note I will be cleaning up the mupen64plus-ae diffs in the next few weeks and pushing them to direct forks of the upstream repositories.  Discussion here for anyone interested.

[Richard Goedeken]

Thanks to everyone who replied on this topic; you all brought up good points
to consider. I can see advantages and disadvantages with each course of
action, but overall I think it's best to keep our current design. I would
prefer not to get back into the position of maintaining a GUI interface for
the emulator again, especially with a separate upstream development group
working on the same GUI for a different and larger project. I also prefer to
keep our own independent API for the existing front-end clients, and not make
them second-class citizens by adopting a new official GUI.

However I also would like to keep Mupen64Plus advancing forward and work with
the libretro developers to make sharing code between our projects as painless
as possible. Daniel, I would like to take you up on your offer to rebase the
current libretro/Mupen64Plus port so that we can merge as many of your changes
as possible and simplify merging back and forth. I would like to avoid major
disruptions to my existing API to keep compatibility with existing Mupen64Plus
front-ends, but I think this can be done relatively easily with the changes
that you mentioned. Our API is versioned and documented, so we can make the
changes in ways that minimally impact other modules. We could add a new
function for handling the state loading/saving through memory buffer pointers
instead of file i/o. I'm fine with the data structure renaming to help
compatibility with your build setup (all plugins in a single shared lib).

I also received a message on emutalk from Zuzma about some good Mupen64Plus
improvements which have been merged into the libretro port which would be good
for us to take. I'll address these in a separate email.

Regards,
Richard

[bobby.s...@gmail.com]

Now that there is no global variables in the hle core, I gave I try to implement some kind of regression testing [1].

Basic idea is:

hle_t state = load_initial_state(initial.bin)
for(i = 0; i < exec_count; ++i)
  hle_execute(initial_state)

hle_t expected_state = load_expected_state(expected.bin)
compare(expected, state)

With sufficient collected "initial / expected" pairs it can help to quickly detect changes in code behavior.

Some implementation details:

-hle.c got 2 more functions for loading/saving hle state from/to file
-I create a static library "librsphle.a" regrouping core hle objects files to ease linking with the regtest program
-creation of "input/expected" hle state can be achieved by inserting HLE_DUMP macro at strategic points, and running the full emulator. (I made an example in alist_audio.c).
-I added basic performance mesuring

Issues/Limitations/Questions:
-Collecting sufficient "initial / expected" pairs of hle state can be a pain.
-It is even worse if/when the hle_t struct is modified (for instance I intend to remove some mp3_* structure members in a (not so?) distant future)
-And unfortunately, this is precisely the situation in which you want to test for regression :(
-Getting a meaningfull regression report from hle_t to hle_t comparison can be hard (ie I can detect that something changed, but it doesn't tell me which function caused that).
-This approach is not really extensible for "real debugging" as this would require a full n64 simulation instead of just calling hle_execute.
-It can only detect "changes" in behavior, not really if the actual behavior is good or wrong (ie I do not compare with an accurate rsp simulator)
[but maybe this should be tackled with another program ?]

If you see value in what I proposed, and wish to integrate that, we can discuss on how to integrate that nicely (I'll need some help there).
Otherwise, it was a fun weekend hacking project (and I should have done it before starting the refactoring) :)

[1] https://github.com/bsmiles32/mupen64plus-rsp-hle/tree/regtest_draft