Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

Flaky android sound fix #15

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

roponator
Copy link

@roponator roponator commented Feb 6, 2017

TLDR: there is an issue when calling C++ functions from C# which have float parameters passed by value: the parameters in the function signature after the float parameters are corrupted. I'm totaly out of ideas, I have not tried compiling mono/xamarin, but it seems as if there is an issues in the C# -> C++ interop layer.
Therefore if a function has more than one float parameters or has a parameter after the float parameter it will get corrupted. The funny thing is that if paramters are of type int it works 100% correctly.

In practice this causes sounds to not be played, because calls like AL.Source that set position (which accepts 3 floats) pass two of the three floats corrupted (their values become like 5*10^38)

This branch is a workaround around the issue as it passes multiple float parameters by reference(pointers).

This fix could be considered totaly "legit" if all functions with float params (even those with just a single float param) would be passed by reference, and not just those with multiple float params, but this seems to cause no issues in practice.

I wrote a detailed issue report on stack overflow and on git:
http://stackoverflow.com/questions/42028853
MonoGame/MonoGame#5443

matevž ropret added 2 commits February 6, 2017 02:34
- replaced the openal C++ interface: replaced functions which passes multiple floats by value with function that pass array of floats. Passing a single or multiple floats by value corrupts all parameters after it, this is a workaround.
@@ -29,11 +29,11 @@

namespace OpenTK
{
#if OPENTK_0
/*#if OPENTK_0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roponator It think this line needs to be uncommented again.

@roponator
Copy link
Author

roponator commented Feb 6, 2017

@dellis1972 If I uncomment this I get this error when compiling:

Severity	Code	Description	Project	File	Line	Suppression State
Error		java.lang.IllegalArgumentException: already added :  Lopentk_1_0/GameViewBase;	MonoGameAndroidAudioTest			

I've done the same in AndroidGameView.cs.

I haven't worked with this 'Register' stuff until now so I'm not sure what's up with it. Could it be some other change which cause it to be reigstered already?

@roponator
Copy link
Author

roponator commented Feb 6, 2017

It seems this is a know problem:
1feb376

Not sure if it's safe to change 1.0 to 1.1 though. Any thoughts?

@dellis1972
Copy link
Contributor

@roponator when debugging the code locally you need to change it to

Register ("opentk_1_0/GameViewBase1")

This is because of the code generator. Its just a side effect of trying to debug. When a release is done (i.e. when we ship OpenTK with the android product) those attributes need to be in place.

@roponator
Copy link
Author

@dellis1972 ok, thx for explaining.

Reverted those files.

@roponator
Copy link
Author

@dellis1972 Hey, you said you work on Xamarin.Android and that you could get someone to take a quick look at this, could you please poke someone so we know if the problem is in Xamarin.Android or somewhere else.

@dellis1972
Copy link
Contributor

@radekdoulik are you able to review this issue/change?

@roponator
Copy link
Author

@dellis1972 @radekdoulik
bump, this thing is still a total mystery, did the xamarin team every experienced anything like it?

Base automatically changed from master to main March 1, 2021 23:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants