Combine mupen64 and mupen64_nogui into one exe?

[ebenblues]

There seems to be a lot of duplicated code between main/main.c (used for mupen64_nogui) and main/gui_gtk/main_gtk.c (used for mupen64). Worse, main.c has some bugs that main_gtk.c does not. For example, on my system, the same video plugin is choppy using mupen64_nogui, but smooth using mupen64. I haven't tracked down the exact cause of this bug, but again, I'm guessing it leads back to something that's already fixed in the gui version. But main.c also has some features (many commandline options) that main_gtk.c does not.

So I'd like to propose combining the two into one mupen64 exe that has a --nogui switch (in addition to all of the other commandline options currently provided by mupen64_nogui). All of the common code would go in main/main.c and main/gui_gtk/main_gtk.c would be used for all of the gtk-specific functions. I would use the current main_gtk.c as the basis for the new common main.c since it's more maintained and seems to work better than the current main.c. Then I'd separate all of the gui-specific code into a new main_gtk.c file.

I just wanted to get people's thoughts on this before I went ahead and started working on the fix.

 

[Richard42]

So I'd like to propose combining the two into one mupen64 exe that has a --nogui switch (in addition to all of the other commandline options currently provided by mupen64_nogui).

I think that's a pretty good idea - it's inconvenient to have separate executables and maintain 2 different 'main' files that do almost the same thing. My primary concern is that everything from the _nogui build gets merged properly. I've done a lot of development on this, and almost exclusively use the _nogui build, so all of the features and fixes need to come along. The recent LIRC code is also only present in the _nogui build.

I can see some small technical issues with this integration (makefiles need to be updated, all the command-line options need to be merged). There are also some not-so-small technical issues. The plugin-loading code is totally different. In the GUI build, the plugins have to be in the same folder, and they are chosen from the drop-down menus in the config dialog, which is populated by scanning a single folder. However, in the NOGUI build, the plugins can be specified via full paths given on the command-line. I don't know if there is any simple and clean way to consolidate these features. I can imagine all kinds of bugs.

 

[Hacktarux]

I also see other advantages of having a separate main file with no gui:
- the binary does not depend at all of gtk
- the source code is a very good starting point for people who want to understand how things are called and start a new port.

 

[ebenblues]

My primary concern is that everything from the _nogui build gets merged properly. I've done a lot of development on this, and almost exclusively use the _nogui build, so all of the features and fixes need to come along. The recent LIRC code is also only present in the _nogui build.

I can see some small technical issues with this integration (makefiles need to be updated, all the command-line options need to be merged). There are also some not-so-small technical issues. The plugin-loading code is totally different. In the GUI build, the plugins have to be in the same folder, and they are chosen from the drop-down menus in the config dialog, which is populated by scanning a single folder. However, in the NOGUI build, the plugins can be specified via full paths given on the command-line. I don't know if there is any simple and clean way to consolidate these features. I can imagine all kinds of bugs.

Thanks for the feedback. I agree with your concerns. My intention is to keep the functionality in a way that makes sense. I think the features in the nogui and the gui version are actually pretty complimentary for the most part. I'll make sure to post any major merge conflicts I find in this thread so we can discuss how we want the program to behave.

I also see other advantages of having a separate main file with no gui:
- the binary does not depend at all of gtk
- the source code is a very good starting point for people who want to understand how things are called and start a new port.

Yeah, I thought about the gtk dependency issue, but I think that can be mitigated using ifdef's and the makefile. Something like 'make nogui_only' would build without gtk dependencies. As for the second point, I'm pretty confident that the new main.c file can be written in a way that's still readable and straightforward.

I'll work on the fix and submit the patch for review before committing it.

 

[Richard42]

I'll work on the fix and submit the patch for review before committing it.

That's going to be a big change affecting several files - it won't be just a small patch. It might be best to make a branch in SVN and work on it incrementally until it's perfect, then we can merge it back in. I can make the branch for you if you haven't done one before.

 

[ebenblues]

That's going to be a big change affecting several files - it won't be just a small patch. It might be best to make a branch in SVN and work on it incrementally until it's perfect, then we can merge it back in. I can make the branch for you if you haven't done one before.

That'd be great. I was thinking a branch would be better, but didn't know how to make one in svn. Let me know when you've made the branch (and how you did it would be nice too).

 

[Richard42]

That'd be great. I was thinking a branch would be better, but didn't know how to make one in svn. Let me know when you've made the branch (and how you did it would be nice too).

Creating the branch is easy - just one command. Merging back into the trunk at the end is a little trickier, but still much easier than CVS. The command that I used to do this locally on the server is:

svn copy svn://localhost/mupen64-amd64/trunk/mupen64-64bit svn://localhost/mupen64-amd64/branches/mupen64-64bit/r199-gui-nogui-merge -m "Created branch for ebenblues to work on merging the gui and nogui builds together"

To do this remotely (which the developers should have authorization to do), just replace the 'localhost' with the hostname and port for my server.

Note that you should be able to use 'svn switch' to switch your local copy onto this branch, without having to check out another copy.

 

[DarkJezter]

This may just be because I'm used to the way it is now, but I still like the idea of having separate executables generated. (the gtk dependency is an issue for me on my mythtv box) At the very least, I agree that there is duplicated code that can be refactored out of each version, so that both the gui and nogui versions share as much code as possible.

Just a thought.

 

[ebenblues]

This may just be because I'm used to the way it is now, but I still like the idea of having separate executables generated. (the gtk dependency is an issue for me on my mythtv box) At the very least, I agree that there is duplicated code that can be refactored out of each version, so that both the gui and nogui versions share as much code as possible.

Just a thought.

Thanks for your post. I also have a mythtv box that I'm using with mupen64 so I'm doing my best to be sensitive to that environment.

I'm writing the combined executable so that it can be built as "nogui only" which will not require gtk dependencies. Also, for those who can't let go of the mupen64_nogui program name, I'm thinking of adding support such that if you create a symlink to the mupen64 program called mupen64_nogui it will act as if you ran mupen64 with the --nogui option.

 

[ebenblues]

Update

Just wanted to post an update on the mupen64 gui/nogui merge. I'm still far from having working code to merge into trunk, but I wanted to post how I'm approaching this from an organizational standpoint:

• I've removed all references to mupen64_nogui in the Makefile. Instead, you can pass the NOGUI_ONLY=1 option to build mupen64 without gui dependencies.
• main/guifuncs.h was an already existing file which looks like it was meant to keep gui code separate from the core code, but it only has a small number of functions that are used in a few places. I think this is a good design and am going to work on fixing the code to follow this design point.
• main/main.[ch] should contain only functional code (no gui-specific code). Any gui-related calls should be calls to functions declared in main/guifuncs.h that are implemented in gui-specific code files.
• All functions declared in main/guifuncs.h will be implemented for gtk in main/gui_gtk/*.[ch] files.
• main/guifuncs.c is a new file I created that defines the functions declared in main/guifuncs.h for the nogui build only. Basically, it defines empty functions for most things, or if there is a function that is supposed to collect information from the user (yes/no dialog), it will prompt the user at the console.

There are many TODO items still, but here's a list of the major ones:

• Implement gtk versions of functions declared in main/guifuncs.h.
• Implement nogui versions of functions declared in main/guifuncs.h (in main/guifuncs.c).
• Move main/gui_gtk/config.[ch] to main/. These are gui-independent utility functions for reading/modifying the mupen64.conf file and should be available to all gui implementations, not just gtk.
• Lots of gui code mixed with functional code in main/gui_gtk which needs to be separated out.

My first priority is to get the gui/functional code separated as described above and get both gui/nogui versions working how they do currently in trunk. Then I will start discussions on some of the design issues that have already been brought up in this forum, e.g., how to handle plugins in gui vs nogui mode.

 

[ebenblues]

gui/nogui merged program working!

I've completed the gui/nogui merge and both the NOGUI_ONLY and regular mupen64 programs build and seem to work correctly. I also added a change so creating a symlink to the mupen64 program called mupen64_nogui will act the same as calling mupen64 with the --nogui option. This is for backwards compatability for people who can't let go of the mupen64_nogui program name.

I need help testing the new code (especially the lirc and vcr code), so for anyone who wants to help test it, you can check out the branch via the following command:

svn co svn://fascination.homelinux.net:7684/mupen64-amd64/branches/mupen64-64bit/r199-gui-nogui-merge

svn username and password are "mupen64" and "Dyson5632-kart", respectively.

After that, cd to r199-gui-nogui-merge/ and run "make all" to build with gui support or "make NOGUI_ONLY=1 all" to build mupen64 without gui support. Please report bugs to this thread.

Also, we need to discuss handling of plugins in the new merged gui/nogui build. What seemed to make the most sense while I was writing this was to accept plugins specified at the commandline and to also scan for plugins in the plugins folder in the working directory. Here's a list of rules that I think make sense. Let me know what you think.

1. If no plugin is specified at the commandline, the plugin in the config file should be the default. The plugin in the config file should also be the one currently selected in the plugin tab of the configure menu.
2. A plugin specified at the commandline should be used instead of the one specified in the config file, but the config file should NOT be modified to use the plugin specified at the commandline.
3. A plugin specified at the commandline should be the selected plugin in the configure menu's plugins tab (even if the config file specifies a different plugin).
4. If the plugin is changed in the configure menu, that plugin should become the plugin that will be used when a rom is played (even if a different plugin was specified at the commandline), and that plugin should be written to the config file so it's the default next time mupen64 is run. Not sure if we want this since the default could be set to a plugin specified at the commandline that is not in the plugins dir, which would mean it might not be found next time mupen64 is run.

Hope those rules made some sense...it's kind of late

 

[Richard42]

I've completed the gui/nogui merge and both the NOGUI_ONLY and regular mupen64 programs build and seem to work correctly.

I'll add a few comments here. The first is unrelated to the plugin issue. I saw in the commit log messages that you modified the _nogui build to run the emulator in a thread, the same way that it works in the GUI build. I just wanted to raise a flag about this. For one thing, this will cause a problem when profiling with Gprof. At work I recently tried profiling a multi-threaded application using Gprof and found that it only profiles the main thread - the other threads don't get profiled. I found a supposed workaround (reading/setting the ITIMER_PROF value) on several websites and tried it, but it didn't work. This isn't a show-stopper, since I've found it better to use OProfile anyway, but it's still unfortunate. The other potential issue is debugging with gdb. I'm not sure how GDB will handle debugging a multi-threaded application. I might be able to find the time to check out the branch and do some testing, but if running the emulator in a thread hinders the ability to use gdb for debugging then it will be a problem for me.

Regarding your 4 suggestions for handling the plugins, I think they're all good ideas. There are other problems that may still remain, however. For example, the config file stores the full path to the plugin, but the drop-down box in the config dialog only shows the name. Even worse, if the plugin path given in the config file does not share the same base directory as the plugins folder which is scanned at startup time, mupen64 will fail to load the plugin. (at least, this was the behavior of mupen64 v0.5 - I can't remember if I fixed it or not) So, if the user moves the executable/plugin folder to a different place and then runs the emulator with a config file which contains the old paths, it will have problems. To get around these sort of issues I would also propose the following changes:

1. Only store the *filenames* for the plugins in the config file, not the full path names
2. Don't even display the plugins specified through the command line in the config dialog box - only display the ones found in the scanned plugins folder.

For the sake of simplicity, I would propose to separate the two plugin selection mechanisms: one is via the command-line; any file can be specified with a full path. The second is through the config file and config dialog box. Keep these mechanisms totally separate, with only a single boolean to decide which one to use. If the command-line option is given, then use that one up until the point where the user selects a plugin via the config dialog. Otherwise, use the plugin given in the config file, until the user select a different one via the config dialog.

 

[ebenblues]

I'll add a few comments here. The first is unrelated to the plugin issue. I saw in the commit log messages that you modified the _nogui build to run the emulator in a thread, the same way that it works in the GUI build. I just wanted to raise a flag about this. For one thing, this will cause a problem when profiling with Gprof.

In the nogui version, I do a pthread_join of the emulation thread after creating the thread, so the main thread waits for the other thread before continuing. I'm not sure how pthread works under the covers, but by the name "pthread_join", could it be possible that the emulation thread gets folded into the main thread? Maybe that doesn't make any sense...I think it's trivial to run the emulation as part of the main thread in nogui mode. I'll go ahead and make that change so this isn't an issue for you.

 

[Günther1]

Also, we need to discuss handling of plugins in the new merged gui/nogui build. What seemed to make the most sense while I was writing this was to accept plugins specified at the commandline and to also scan for plugins in the plugins folder in the working directory. Here's a list of rules that I think make sense. Let me know what you think.

I'd suggest just disabling the dropdown box if a plugin has been specified on the command line, to keep the complexity down. If someone can specify plugins on the command line, he also can rerun the emulator without the parameters if he wants to use the gui list.

 

[ebenblues]

I'd suggest just disabling the dropdown box if a plugin has been specified on the command line, to keep the complexity down. If someone can specify plugins on the command line, he also can rerun the emulator without the parameters if he wants to use the gui list.

I like that idea, but I think it should still be possible for the user to configure the plugin. Maybe I could make it so the commandline plugin is the one selected in the drop down, but the user can't change the selected plugin. That way they'd still be able to configure it. I'll play around with it and see what seems to make sense. Thanks for the suggestion!

 

[Günther1]

Maybe I could make it so the commandline plugin is the one selected in the drop down, but the user can't change the selected plugin. That way they'd still be able to configure it.

That's what I meant.

 

[ebenblues]

merge complete

I just committed changes to handle plugins specified at the commandline in the gui version of mupen64. Thanks to Günther for simplifying the design.

So...that was the last item on my TODO list for the gui/nogui merge! I've made a lot of significant changes, but I'm really satisfied with the results of the merge. I was able to cleanup a lot of areas of the code. lirc support still needs to be tested to verify I didn't break anything, but other than that, I think it's ready to merge into trunk.

Richard42, what are your thoughts? I'm starting to make non gui/nogui-merge related fixes in this branch and I think it'd be best to make those in trunk after the merge. I've been following the commit mail and I haven't really seen any changes to the areas of code I've been working on, so the merge to trunk might be pretty painless.

 

[Richard42]

Richard42, what are your thoughts? I'm starting to make non gui/nogui-merge related fixes in this branch and I think it'd be best to make those in trunk after the merge. I've been following the commit mail and I haven't really seen any changes to the areas of code I've been working on, so the merge to trunk might be pretty painless.

I was thinking that it's about time to consider stabilizing the branch and merging it back in, since you've committed so many changes. The only thing that I worry about a little is that NMN merged in a change in rev 213 to handle small/large icons in the GUI. It will probably conflict with your changes. It shouldn't be too hard to merge though I guess.

Is the branch stable; are there any outstanding issues? If not, I can do the merge this weekend. Do you have the time to create a README file in the root directory and document the command-line options and the general usage of the new unified executable and installation procedure? It would be nice to have some simple doc there so people will know what to expect.

After this is done we'll have to go into bug fixing mode for a few weeks and get everything ironed out in preparation for the next release. We'll have another 2 major features: the glide64 video plugin and the unified gui/nogui system, plus a bunch of bug fixes. That should be a good chunk for a new release, and we'll use the new name. Unless anyone has a better idea, I think Mupen64Plus is ahead in the polls now.

 

[ebenblues]

I was thinking that it's about time to consider stabilizing the branch and merging it back in, since you've committed so many changes. The only thing that I worry about a little is that NMN merged in a change in rev 213 to handle small/large icons in the GUI. It will probably conflict with your changes. It shouldn't be too hard to merge though I guess.

Ah, I missed nmn's change in the commit mail. Looking through the patch file, it doesn't look like it'll be a painful merge. I don't think I touched the parts of the functions that nmn modified. I would recommend applying his changes to the copies of the files in the branch and then just copying the files into trunk, but I haven't worked with svn branches before this, so you might know a better way. Btw, is there a way for me to view the changes for a particular revision using tkdiff?

Is the branch stable; are there any outstanding issues? If not, I can do the merge this weekend. Do you have the time to create a README file in the root directory and document the command-line options and the general usage of the new unified executable and installation procedure? It would be nice to have some simple doc there so people will know what to expect.

There are no outstanding issues (that weren't there before I did the merge) in the testing I've done on my machine. In fact, the merge actually fixed a slow framerate problem I was having with the nogui exe before the merge. The only thing I haven't tested is lirc support, but I'm sure you'll let me know if there's an issue quickly after the merge. If you could do the merge this weekend, that'd be great. I'll work on getting a README file together.

 

[GuillaumeH]

I need help testing the new code (especially the lirc and vcr code), so for anyone who wants to help test it, you can check out the branch via the following command:

svn co svn://fascination.homelinux.net:7684/mupen64-amd64/branches/mupen64-64bit/r199-gui-nogui-merge

svn username and password are "mupen64" and "Dyson5632-kart", respectively.

It's not particularly on-topic right now, but maybe you could benefit from using a SVN + issue tracker system website to ease the development of Mupen64. You should take a look at http://code.google.com/hosting/ for instance, who has a very nice and simple issue tracker, and a can host a SVN repository.