Forwarding on to wine-devel, Stas, and Ruslan from Bug 38041.
Any thoughts on the coning breakage, Stas?
Andrew
On Mon, Mar 23, 2015 at 09:52:32PM +0000, Mark Harmstone wrote:
Hi,
I've had a play with it, and yes, I think he's right. It seems to be a lot more robust than my code. One thing though that I think he's overlooked is that he changes the behaviour of AngleBetweenVectorsRad, which is called by AngleBetweenVectorsDeg and thus the coning code - I think this patch will probably break that.
I've not tested it yet, but there's a chance this patch might fix bug 38041.
Mark
On 23/03/15 14:39, Andrew Eikum wrote:
This seems sane to me, but I never had a good grasp on this to begin with. Does it look OK to you, Mark?
Andrew
On Sun, Mar 22, 2015 at 08:59:15PM +0300, Стас Цымбалов wrote:
This patch fixes incorrect sound positioning in dsound. As of now, angle to sound source is calculated as angle between vDistance and vOrientFront. This leads to incorrect results for all sources that are not in the same plane as speakers. In extreme case: for sources directly above or below origin, angle is calculated to -90 degrees, and they are played in the left speaker.
This patch changes angle calculation: angle to sound source is calculated as angle between vDistance and plane given by vectors vOrientFront and vOrientTop.
dlls/dsound/sound3d.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
From 1ebe3761b0d6acfca58cec17472336a05b7f2679 Mon Sep 17 00:00:00 2001 From: Stas Cymbalov dummyunit@gmail.com Date: Sun, 22 Mar 2015 18:22:49 +0300 Subject: dsound: Fix angle to sound source calculation.
dlls/dsound/sound3d.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/dlls/dsound/sound3d.c b/dlls/dsound/sound3d.c index 9a0226a..ffd4d45 100644 --- a/dlls/dsound/sound3d.c +++ b/dlls/dsound/sound3d.c @@ -112,7 +112,6 @@ static inline D3DVALUE AngleBetweenVectorsRad (const D3DVECTOR *a, const D3DVECT
cos = product/(la*lb); angle = acos(cos);
- if (cos < 0.0f) { angle -= M_PI; } TRACE("angle between (%f,%f,%f) and (%f,%f,%f) = %f radians (%f degrees)\n", a->x, a->y, a->z, b->x, b->y, b->z, angle, RadToDeg(angle)); return angle;
@@ -264,16 +263,20 @@ void DSOUND_Calc3DBuffer(IDirectSoundBufferImpl *dsb) else { vLeft = VectorProduct(&dsb->device->ds3dl.vOrientFront, &dsb->device->ds3dl.vOrientTop);
flAngle = AngleBetweenVectorsRad(&dsb->device->ds3dl.vOrientFront, &vDistance);flAngle2 = AngleBetweenVectorsRad(&vLeft, &vDistance);/* AngleBetweenVectorsRad performs a dot product, which gives us the cosine of the angle* between two vectors. Unfortunately, because cos(theta) = cos(-theta), we've no idea from* this whether the sound is to our left or to our right. We have to perform another dot* product, with a vector at right angles to the initial one, to get the correct angle.* The angle should be between -180 degrees and 180 degrees. */if (flAngle < 0.0f) { flAngle += M_PI; }if (flAngle2 > 0.0f) { flAngle = -flAngle; }
/* To calculate angle to sound source we need to:* 1) Get angle between vDistance and a plane on which angle to sound source should be 0.* Such a plane is given by vectors vOrientFront and vOrientTop, and angle between vector* and a plane equals to M_PI_2 - angle between vector and normal to this plane (vLeft in this case).* 2) Determine if the source is behind or in front of us by calculating angle between vDistance* and vOrientFront.*/flAngle = AngleBetweenVectorsRad(&vLeft, &vDistance);flAngle2 = AngleBetweenVectorsRad(&dsb->device->ds3dl.vOrientFront, &vDistance);if (flAngle2 > M_PI_2)flAngle = -flAngle;flAngle -= M_PI_2;if (flAngle < -M_PI)flAngle += 2*M_PI;-- 2.0.5
As I've tested, this does indeed fix bug 38041.
On Tue, Mar 24, 2015 at 7:31 PM, Andrew Eikum aeikum@codeweavers.com wrote:
Forwarding on to wine-devel, Stas, and Ruslan from Bug 38041.
Any thoughts on the coning breakage, Stas?
Andrew
On Mon, Mar 23, 2015 at 09:52:32PM +0000, Mark Harmstone wrote:
Hi,
I've had a play with it, and yes, I think he's right. It seems to be a lot more robust than my code. One thing though that I think he's overlooked is that he changes the behaviour of AngleBetweenVectorsRad, which is called by AngleBetweenVectorsDeg and thus the coning code - I think this patch will probably break that.
I've not tested it yet, but there's a chance this patch might fix bug 38041.
Mark
On 23/03/15 14:39, Andrew Eikum wrote:
This seems sane to me, but I never had a good grasp on this to begin with. Does it look OK to you, Mark?
Andrew
On Sun, Mar 22, 2015 at 08:59:15PM +0300, Стас Цымбалов wrote:
This patch fixes incorrect sound positioning in dsound. As of now, angle to sound source is calculated as angle between vDistance and vOrientFront. This leads to incorrect results for all sources that are not in the same plane as speakers. In extreme case: for sources directly above or below origin, angle is calculated to -90 degrees, and they are played in the left speaker.
This patch changes angle calculation: angle to sound source is calculated as angle between vDistance and plane given by vectors vOrientFront and vOrientTop.
dlls/dsound/sound3d.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
From 1ebe3761b0d6acfca58cec17472336a05b7f2679 Mon Sep 17 00:00:00 2001 From: Stas Cymbalov dummyunit@gmail.com Date: Sun, 22 Mar 2015 18:22:49 +0300 Subject: dsound: Fix angle to sound source calculation.
dlls/dsound/sound3d.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/dlls/dsound/sound3d.c b/dlls/dsound/sound3d.c index 9a0226a..ffd4d45 100644 --- a/dlls/dsound/sound3d.c +++ b/dlls/dsound/sound3d.c @@ -112,7 +112,6 @@ static inline D3DVALUE AngleBetweenVectorsRad (const D3DVECTOR *a, const D3DVECT
cos = product/(la*lb); angle = acos(cos);
- if (cos < 0.0f) { angle -= M_PI; } TRACE("angle between (%f,%f,%f) and (%f,%f,%f) = %f radians (%f degrees)\n", a->x, a->y, a->z, b->x, b->y, b->z, angle, RadToDeg(angle)); return angle;
@@ -264,16 +263,20 @@ void DSOUND_Calc3DBuffer(IDirectSoundBufferImpl *dsb) else { vLeft = VectorProduct(&dsb->device->ds3dl.vOrientFront, &dsb->device->ds3dl.vOrientTop);
flAngle = AngleBetweenVectorsRad(&dsb->device->ds3dl.vOrientFront, &vDistance);flAngle2 = AngleBetweenVectorsRad(&vLeft, &vDistance);/* AngleBetweenVectorsRad performs a dot product, which gives us the cosine of the angle* between two vectors. Unfortunately, because cos(theta) = cos(-theta), we've no idea from* this whether the sound is to our left or to our right. We have to perform another dot* product, with a vector at right angles to the initial one, to get the correct angle.* The angle should be between -180 degrees and 180 degrees. */if (flAngle < 0.0f) { flAngle += M_PI; }if (flAngle2 > 0.0f) { flAngle = -flAngle; }
/* To calculate angle to sound source we need to:* 1) Get angle between vDistance and a plane on which angle to sound source should be 0.* Such a plane is given by vectors vOrientFront and vOrientTop, and angle between vector* and a plane equals to M_PI_2 - angle between vector and normal to this plane (vLeft in this case).* 2) Determine if the source is behind or in front of us by calculating angle between vDistance* and vOrientFront.*/flAngle = AngleBetweenVectorsRad(&vLeft, &vDistance);flAngle2 = AngleBetweenVectorsRad(&dsb->device->ds3dl.vOrientFront, &vDistance);if (flAngle2 > M_PI_2)flAngle = -flAngle;flAngle -= M_PI_2;if (flAngle < -M_PI)flAngle += 2*M_PI;-- 2.0.5
Any thoughts on the coning breakage, Stas?
To me it seems that coning should work the same as before commit 02f9edfd77302eabc0a8a6e45a9423ebe2b1acef (which introduced panning between multiple speakers). In that commit AngleBetweenVectorsRad was changed in the way that it is now and there was no comment about how it would affect coning code. So there are 2 possibilities: 1. Coning was broken before 02f9edfd77302eabc0a8a6e45a9423ebe2b1acef, got silently fixed by that commit, and will be broken again after my patch. 2. Coning was fine before 02f9edfd77302eabc0a8a6e45a9423ebe2b1acef, and will be the same after my patch. Which of these is true I don't know, I would appreciate if anybody could point out to me how could I test it.
2015-03-24 19:31 GMT+03:00 Andrew Eikum aeikum@codeweavers.com:
Forwarding on to wine-devel, Stas, and Ruslan from Bug 38041.
Any thoughts on the coning breakage, Stas?
Andrew
On Mon, Mar 23, 2015 at 09:52:32PM +0000, Mark Harmstone wrote:
Hi,
I've had a play with it, and yes, I think he's right. It seems to be a lot more robust than my code. One thing though that I think he's overlooked is that he changes the behaviour of AngleBetweenVectorsRad, which is called by AngleBetweenVectorsDeg and thus the coning code - I think this patch will probably break that.
I've not tested it yet, but there's a chance this patch might fix bug 38041.
Mark
On 23/03/15 14:39, Andrew Eikum wrote:
This seems sane to me, but I never had a good grasp on this to begin with. Does it look OK to you, Mark?
Andrew
On Sun, Mar 22, 2015 at 08:59:15PM +0300, Стас Цымбалов wrote:
This patch fixes incorrect sound positioning in dsound. As of now, angle to sound source is calculated as angle between vDistance and vOrientFront. This leads to incorrect results for all sources that are not in the same plane as speakers. In extreme case: for sources directly above or below origin, angle is calculated to -90 degrees, and they are played in the left speaker.
This patch changes angle calculation: angle to sound source is calculated as angle between vDistance and plane given by vectors vOrientFront and vOrientTop.
dlls/dsound/sound3d.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
From 1ebe3761b0d6acfca58cec17472336a05b7f2679 Mon Sep 17 00:00:00 2001 From: Stas Cymbalov dummyunit@gmail.com Date: Sun, 22 Mar 2015 18:22:49 +0300 Subject: dsound: Fix angle to sound source calculation.
dlls/dsound/sound3d.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/dlls/dsound/sound3d.c b/dlls/dsound/sound3d.c index 9a0226a..ffd4d45 100644 --- a/dlls/dsound/sound3d.c +++ b/dlls/dsound/sound3d.c @@ -112,7 +112,6 @@ static inline D3DVALUE AngleBetweenVectorsRad (const D3DVECTOR *a, const D3DVECT
cos = product/(la*lb); angle = acos(cos);
- if (cos < 0.0f) { angle -= M_PI; } TRACE("angle between (%f,%f,%f) and (%f,%f,%f) = %f radians (%f degrees)\n", a->x, a->y, a->z, b->x, b->y, b->z, angle, RadToDeg(angle)); return angle;
@@ -264,16 +263,20 @@ void DSOUND_Calc3DBuffer(IDirectSoundBufferImpl *dsb) else { vLeft = VectorProduct(&dsb->device->ds3dl.vOrientFront, &dsb->device->ds3dl.vOrientTop);
flAngle = AngleBetweenVectorsRad(&dsb->device->ds3dl.vOrientFront, &vDistance);flAngle2 = AngleBetweenVectorsRad(&vLeft, &vDistance);/* AngleBetweenVectorsRad performs a dot product, which gives us the cosine of the angle* between two vectors. Unfortunately, because cos(theta) = cos(-theta), we've no idea from* this whether the sound is to our left or to our right. We have to perform another dot* product, with a vector at right angles to the initial one, to get the correct angle.* The angle should be between -180 degrees and 180 degrees. */if (flAngle < 0.0f) { flAngle += M_PI; }if (flAngle2 > 0.0f) { flAngle = -flAngle; }
/* To calculate angle to sound source we need to:* 1) Get angle between vDistance and a plane on which angle to sound source should be 0.* Such a plane is given by vectors vOrientFront and vOrientTop, and angle between vector* and a plane equals to M_PI_2 - angle between vector and normal to this plane (vLeft in this case).* 2) Determine if the source is behind or in front of us by calculating angle between vDistance* and vOrientFront.*/flAngle = AngleBetweenVectorsRad(&vLeft, &vDistance);flAngle2 = AngleBetweenVectorsRad(&dsb->device->ds3dl.vOrientFront, &vDistance);if (flAngle2 > M_PI_2)flAngle = -flAngle;flAngle -= M_PI_2;if (flAngle < -M_PI)flAngle += 2*M_PI;-- 2.0.5
On Tue, Mar 24, 2015 at 09:13:43PM +0300, Стас Цымбалов wrote:
Any thoughts on the coning breakage, Stas?
To me it seems that coning should work the same as before commit 02f9edfd77302eabc0a8a6e45a9423ebe2b1acef (which introduced panning between multiple speakers). In that commit AngleBetweenVectorsRad was changed in the way that it is now and there was no comment about how it would affect coning code. So there are 2 possibilities:
- Coning was broken before
02f9edfd77302eabc0a8a6e45a9423ebe2b1acef, got silently fixed by that commit, and will be broken again after my patch. 2. Coning was fine before 02f9edfd77302eabc0a8a6e45a9423ebe2b1acef, and will be the same after my patch. Which of these is true I don't know, I would appreciate if anybody could point out to me how could I test it.
Mark, any thoughts? We could remove the logic from AngleBetweenVectorsRad and add it back just to the coning handling in DSOUND_Calc3DBuffer if you think it's correct.
I've tried working through the logic, but I can't figure out why we want to subtract 180 degrees when the cosine is negative. It seems to me that Stas's change to AngleBetweenVectorsRad is correct. Am I missing something?
Andrew
On 26/03/15 15:26, Andrew Eikum wrote:
On Tue, Mar 24, 2015 at 09:13:43PM +0300, Стас Цымбалов wrote:
Any thoughts on the coning breakage, Stas?
To me it seems that coning should work the same as before commit 02f9edfd77302eabc0a8a6e45a9423ebe2b1acef (which introduced panning between multiple speakers). In that commit AngleBetweenVectorsRad was changed in the way that it is now and there was no comment about how it would affect coning code. So there are 2 possibilities:
- Coning was broken before
02f9edfd77302eabc0a8a6e45a9423ebe2b1acef, got silently fixed by that commit, and will be broken again after my patch. 2. Coning was fine before 02f9edfd77302eabc0a8a6e45a9423ebe2b1acef, and will be the same after my patch. Which of these is true I don't know, I would appreciate if anybody could point out to me how could I test it.
Mark, any thoughts? We could remove the logic from AngleBetweenVectorsRad and add it back just to the coning handling in DSOUND_Calc3DBuffer if you think it's correct.
I've tried working through the logic, but I can't figure out why we want to subtract 180 degrees when the cosine is negative. It seems to me that Stas's change to AngleBetweenVectorsRad is correct. Am I missing something?
Andrew
That's entirely possible - I can't remember if I touched the coning code when I put in the surround sound support. Do you know if there's any testcase we could use for coning support?
Mark
-
Andrew Eikum -
Mark Harmstone -
Ruslan Kabatsayev -
Стас Цымбалов