Results of a small investigation into the plugins.

Feedback about snapshots

Results of a small investigation into the plugins.

Postby rawnar » Wed Oct 20, 2010 11:55 am

I have had a look in the source code of the Positional Audio plug-ins to check the calculations. While I was going through them I also found some other thinks that looked off. I will list them here in three sections. Errors these are mistakes that can have effect on the usage of the plug-ins. Improvements things that can be changed to optimize the plug-ins. Questions some part of the code I did not understand. Each item will begin with a list of plug-ins that are related to the comment.

ERRORS:
  • cod4, cod5, codmw2, etqw, wolfet
  • Fetch() is used to check the function of the plug-in before sending true back in trylock(). Only the fetch function has a width range of values for the state check that will cause the function to return true. So different versions of the executable will return true for trylock(). This can result in a strange behaviour of the plug-in.
  • bf2142, bfbc2, bfheroes
  • The avatar vectors are filled before making a clean exit when the state vectors says your are not in a map. The avatar vector will still be send via the server to the other users in the channel. So if someone else in the channel is in the map he will hear you positionally while you are not in the map.
  • l4d, l4d2
  • Sting assignments within a for loop. The context and identity string are assigned three times with the same value.
  • arma2
  • Checking a variable before you have checked if the variable was read correctly by peekProc().
  • bfbc2
  • Call to generic_unlock() in the fetch function. This seems incorrect, when compared to other plug-ins.
  • etqw
  • According to the comments the first and last entry of the front vector are incorrectly calculated. They need to be avatar_front[0] = cos(vievHor) * cos(viewVer); and avatar_front[2] = sin(viewHor) * cos(viewVer);
  • lotro
  • According to the comments the axis system is already left-handed, but it is made right-handed by switching to axis. The axis needs to by cycled. To get the y-axis pointing upward use, x->z, y->x, z->y. For the heading I can not give advice as I can not find comments how the angle is defined. (Talked to the coder and the comments in the source were wrong, so this remark can scratched)

IMPROVEMENTS:
  • aoc, bf2, cod4, cs, css, dods, dys, etqw, gmod, hl2dm, insurgency, tf2, ut2004, ut3
  • The namespace std is used in the code. I did not put this in the error section, but some coders probably think it would be.
  • aoc, css, dods, dys, gmod, hl2dm, insurgency, tf2
  • In fetch() move the string handling after possible quick escapes. The strings are filled while in a later part of the function there is a check that can exit the function with false.
  • arma2, lotro, wow
  • In these plugins there are several variables, functions or included headers that are not used. Cleaning up the code will make them better readable.
  • cod2, cod4, cod5, codmw2, codmw2so, etqw, wolfet
  • When angle are used to calculated the front vector also calculated the top vector and not provide a top vector which is almost never orthogonal to the front vector. The positional audio will work, but mumble has to recreate the top vector be finding the azimuth and inclination angle.
  • arma2, bf1942, ut2004
  • There is not check if one is in a map or not. This can result in strange behaviour of the plugin. I have not put it in the error section, as I know how hard it can be to find a good state value.;-)

QUESTIONS:
  • aoc, cs, css, dods, dys, gmod, hl2dm, insurgency, l4d, l4d2,tf2
  • The angles to calculate the front and top vector are checked, but why is that specific range picked? The range for the angles are -360<v<360 and -360<h<360, and for the cs plugin it is even -720<v<720 and -720<h<720. It will work as sin and cos are periodic, but why not have a range that spans 360 degrees for v and 180 degrees for h?
  • bf2, bf2142, bfheroes
  • Why return the fetch function with false when the user is not logged in? Are the pointers not working or is there no option to play a multiplayer game when you are not loggedin?

There you have it, another walk of text by the one and only forum spammer Mr. Rawnar. :D
Last edited by rawnar on Thu Nov 25, 2010 8:36 am, edited 4 times in total.
Computer specs: AMD FX-8320, 8GB DDR3-SDRAM, AMD Radeon HD 7950, Asus Xonar D1, Windows 7 Ultimate 64bit/Debian Jessie AMD64.
rawnar
 
Posts: 243
Joined: Tue Mar 09, 2010 8:54 am
Location: Borne, the Netherlands

Re: Results of a small investigation into the plugins.

Postby rawnar » Wed Oct 20, 2010 6:50 pm

Probably the first error, which is about the call to fetch within the trylock function, his not so prudent. Maybe we only need to be sure that the executable we are reading data from is the version we expect it to be. What I mean is do a simple peekProc on a string, so it can be checked against something expected. This way you know that you are connected to the executable you have written the plugin for. The call for fetch is only an additional test.
Last edited by rawnar on Thu Oct 21, 2010 8:26 am, edited 1 time in total.
Computer specs: AMD FX-8320, 8GB DDR3-SDRAM, AMD Radeon HD 7950, Asus Xonar D1, Windows 7 Ultimate 64bit/Debian Jessie AMD64.
rawnar
 
Posts: 243
Joined: Tue Mar 09, 2010 8:54 am
Location: Borne, the Netherlands

Re: Results of a small investigation into the plugins.

Postby rawnar » Thu Oct 21, 2010 8:26 am

Going through the plug-ins again showed me that only cod4, cod5, codmw2, etqw and wolfet need an addition check in the trylock before returning true. In the fetch() call the state value is check to be unequal to a single number before returning true. Which means a large range of numbers will return true, so there is no good test on the executable version.

The initial post has been updated.
Computer specs: AMD FX-8320, 8GB DDR3-SDRAM, AMD Radeon HD 7950, Asus Xonar D1, Windows 7 Ultimate 64bit/Debian Jessie AMD64.
rawnar
 
Posts: 243
Joined: Tue Mar 09, 2010 8:54 am
Location: Borne, the Netherlands

Re: Results of a small investigation into the plugins.

Postby hacst » Thu Oct 21, 2010 3:27 pm

Thanks for reviewing the plugins. With the corrections everything seems to be valid to me. I guess all that's left now is fixing the code :lol:

ERRORS
As point one in the errors category requires you to have those games (installed) it is the most tricky one. I guess Snares has most of them and maybe we can get Timer to do Cod 5. I could install wolfet and cod4 if no one else steps up.

The rest, at least if the comments in the lotro plugin are correct, are valid catches and should be easy fixes. We just have decide on who does what.

IMPROVEMENTS
1. Doesn't strike me as very problematic but since its a simple string replacement thing can be easily fixed
2. Since the plugin unlinks I'm not really sure whether the string updates do still matter. But for the sake of good style the assigments should be moved, you are right.
3. Ok
4. When I wrote those (cod2/4/wolfet were my first contributions to mumble iirc :roll: ) I left out the top vectors since Mumble got them right on its own. I guess they could be added though I'm not quite sure what this would gain. No implicit behaviour?

Thanks again for taking time to review those.

ps: I was just writing to you about that when you left in IRC and yesterday I couldn't answer you because you left before I was back at my computer ;-) You should think about setting up a bouncer or something like that. Helps IRC communications greatly.
hacst
Team member
Team member
 
Posts: 338
Joined: Wed Sep 23, 2009 4:28 pm

Re: Results of a small investigation into the plugins.

Postby rawnar » Thu Oct 21, 2010 9:05 pm

IMPROVEMENTS
4. If you look at the code in mumble you will see that if the top vector is not perpendicular to the front vector it will recalculate the top vector. Recreating the top vector involves using the front vector together with acos() and atan2() to get the angles, then rotate the colatitude(called inclination in mumble) by 90 degrees and call the top vector with cos() and sin(). So the improvement lies in the fact that the calls to acos() and atan2() will not be needed. It is not much of an improvement as the two function calls will probably only take 100 clock cycles, but adding the top to the plugin is easy when you have the angles. ;)

The games I own are bf2, etqw, cs, css, tf2, ut3. And as far as I know wolfet, dys and insurgency are free.
Computer specs: AMD FX-8320, 8GB DDR3-SDRAM, AMD Radeon HD 7950, Asus Xonar D1, Windows 7 Ultimate 64bit/Debian Jessie AMD64.
rawnar
 
Posts: 243
Joined: Tue Mar 09, 2010 8:54 am
Location: Borne, the Netherlands

Re: Results of a small investigation into the plugins.

Postby kissaki » Thu Oct 21, 2010 10:18 pm

rawnar wrote:dys and insurgency are free.

Free mods, you’ll need a bought source engine game. But dd0t has one as well. :)

I got a lot of those games as well, so feel free to ask if you need help with one of them.
No CoD5 though.
MumPI: Your Mumble Web Interface in PHP
User avatar
kissaki
Team member
Team member
 
Posts: 1262
Joined: Sat Jan 09, 2010 12:15 pm

Re: Results of a small investigation into the plugins.

Postby rawnar » Wed Nov 03, 2010 10:57 am

Additional note on improvement 4. You can also calculate the top vector without the use of acos and atan2. There is only one call to sqrt needed. If the values of the front vector are (x_f, y_f, z_f) then the top vector values are,
Code: Select all
x_t = -x_f * y_f / sqrt(x_f * x_f + z_f * z_f) = -x_f * y_f / y_t
y_t =  sqrt(x_f * x_f + z_f * z_f) = sqrt(1 - y_f * y_f)
z_t = -z_f * y_f / sqrt(x_f * x_f + z_f * z_f) = -z_f * y_f / y_t

Also the right vector can be calculated this way,
Code: Select all
x_r = z_f / sqrt(x_f * x_f + z_f * z_f) = z_f / y_t
y_r =  0
z_r = -x_f / sqrt(x_f * x_f + z_f * z_f) = -x_f / y_t

The zero value of y_r is because this vector will always be in the xy-plane. These formulas only hold when the front vector is a unit vector, which means x_f * x_f + y_f * y_f + z_f * z_f = 1. This method will show instability when the front vector point upwards as y_t will be close to zero. But this also holds for the atan2 method as atan2 is undefined when the first and second argument are zero.

An additional note on the implementation of the positional audio. The values of the positional vector are used to check if positional audio is performed. In my opinion using the front vector for that is a better choice. There are two reason. The first one is, the position vector can contain all zero's while you are in a map, but the front vector always non-zero in length. The change seems small, but if one uses the manual plug-in and forgets to set his position other then the default it will not work. This can cause confusion with the user. The second one is, if the front vector is zero but not the position vector the front and top vector are recreated in a fixed direction. This will mean that a when you are in-game and the person seems to be at your right. Turning your head to the right, the audio will not shift to the front but stay at the right. In other word the positional audio is useless.

P.S. Using the front vector for the check, can help plug-ins that use the avatars_pos vector to check if they are on a map or not.

And the wall keeps on growing. :D
Computer specs: AMD FX-8320, 8GB DDR3-SDRAM, AMD Radeon HD 7950, Asus Xonar D1, Windows 7 Ultimate 64bit/Debian Jessie AMD64.
rawnar
 
Posts: 243
Joined: Tue Mar 09, 2010 8:54 am
Location: Borne, the Netherlands


Return to Snapshots

Who is online

Users browsing this forum: No registered users and 2 guests