- Wine-GitLab - list.winehq.org

Re: [PATCH v2 0/3] MR2727: hrtfapo: Add CreateHrtfApo() stub.
by Zebediah Figura (＠zfigura) May 1, 2023

May 1, 2023

Zebediah Figura (@zfigura) commented about include/hrtfapoapi.idl: > + HrtfDirectivity *directivity; > +} HrtfApoInit; > + > +cpp_quote("STDAPI CreateHrtfApo(const HrtfApoInit *init, IXAPO **xapo);") > + > +[ > + object, > + uuid(15b3cd66-e9de-4464-b6e6-2bc3cf63d455), > + pointer_default(unique) > +] > +interface IXAPOHrtfParameters : IUnknown > +{ > + HRESULT SetSourcePosition([in] const HrtfPosition *position); > + HRESULT SetSourceOrientation([in] const HrtfOrientation *orientation); > + HRESULT SetSourceGain([in] float gain); > + HRESULT SetEnvironment([in] HrtfEnvironment environment); You don't need these [in] attributes; they only matter for remoting. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2727#note_31598

1 0

Re: [PATCH v2 0/3] MR2727: hrtfapo: Add CreateHrtfApo() stub.
by Zebediah Figura (＠zfigura) May 1, 2023

May 1, 2023

Zebediah Figura (@zfigura) commented about include/hrtfapoapi.idl: > + float unityGainDistance; > + float cutoffDistance; > +} HrtfDistanceDecay; > + > +typedef struct HrtfApoInit > +{ > + HrtfDistanceDecay *distanceDecay; > + HrtfDirectivity *directivity; > +} HrtfApoInit; > + > +cpp_quote("STDAPI CreateHrtfApo(const HrtfApoInit *init, IXAPO **xapo);") > + > +[ > + object, > + uuid(15b3cd66-e9de-4464-b6e6-2bc3cf63d455), > + pointer_default(unique) You don't need the pointer_default(unique) attribute; it's the default. Yes, it's everywhere in Wine headers, but it's really unnecessary. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2727#note_31597

1 0

Re: [PATCH v2 0/3] MR2727: hrtfapo: Add CreateHrtfApo() stub.
by Zebediah Figura (＠zfigura) May 1, 2023

May 1, 2023

Zebediah Figura (@zfigura) commented about dlls/hrtfapo/main.c: > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA > + */ > + > +#include "hrtfapoapi.h" > +#include "wine/debug.h" > + > +WINE_DEFAULT_DEBUG_CHANNEL(hrtfapo); Let's use the existing xaudio2 channel here. And in general, matching the style used in xaudio2 would probably be good. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2727#note_31599

1 0

Re: [PATCH v2 0/3] MR2727: hrtfapo: Add CreateHrtfApo() stub.
by Zebediah Figura (＠zfigura) May 1, 2023

May 1, 2023

Zebediah Figura (@zfigura) commented about include/hrtfapoapi.idl: > +typedef struct HrtfDistanceDecay > +{ > + HrtfDistanceDecayType type; > + float maxGain; > + float minGain; > + float unityGainDistance; > + float cutoffDistance; > +} HrtfDistanceDecay; > + > +typedef struct HrtfApoInit > +{ > + HrtfDistanceDecay *distanceDecay; > + HrtfDirectivity *directivity; > +} HrtfApoInit; > + > +cpp_quote("STDAPI CreateHrtfApo(const HrtfApoInit *init, IXAPO **xapo);") You can declare this directly in the IDL. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2727#note_31596

1 0

[PATCH 0/4] MR2726: wine{alsa,coreaudio,oss,pulse}: Move SimpleAudioVolume into mmdevapi.
by Davide Beatrici (＠davidebeatrici) April 29, 2023

April 29, 2023

Please note that with this MR the following `TRACE()` calls are removed: - `"ALSA does not support volume control\n"` - `"OSS doesn't support setting volume\n"` - `"PulseAudio does not support session volume control\n"` Should I perhaps move them to the specific drivers' unixlib instead? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2726

3 6

[PATCH v2 0/1] MR189: tests: Add a test for arrays with an expression as the index.
by Ethan Lee (＠flibitijibibo) April 29, 2023

April 29, 2023

See https://bugs.winehq.org/show_bug.cgi?id=54832 I'm starting to see this particular FIXME in quite a few games (Escape Goat 2, Murder Miners, and Little Racers STREET to name a few), and since I'm not sure how to fix this I figured I could at least provide a test for someone that knows more SM4 than me :P -- v2: tests: Add a test for arrays with an expression as the index. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/189

2 1

Re: [PATCH v3 0/1] MR499: server: Allow cancelling alerted asyncs.
by Jinoh Kang (＠iamahuman) April 29, 2023

April 29, 2023

On Sat Apr 29 14:54:15 2023 +0000, Jinoh Kang wrote: > To reduce complexity, I've tried refactoring the state management. > Currently, the async has the following two state flags that deal with > the state of the client that originally requested the async: > - `signaled`: indicates whether the async object is signaled; the client > may wait on the async handle before returning from the corresponding > wine system call. > - Valid transition: 0 -> 1. > - This flag is set if only if any of the following conditions are true: > - Given that `!async->blocking`, > - `async_wake_obj(async)` has been called when > `async->unknown_status = 0`, or > - `async_handoff(async, ...)` has been called with `get_error() != > STATUS_ALERTED && (async->pending || !NT_ERROR(get_error()) && > (get_error() != STATUS_PENDING || async->iosb->status != STATUS_PENDING)`. > - (Regardless of `async->blocking`) `async_set_result(async, status, > ...)` has been called with `async->terminated && (!async->alerted || > status != STATUS_PENDING)`. > - Note that this flag exhibits slightly different behavior depending > on `async->blocking`. > - If `async->blocking`, it tries to block the requestor until the > async operation has completed. > - If `!async->blocking`, it tries to signal as soon as the initial > status is known, _unless_ the wait handle is being closed, in which case > it doesn't bother to touch the flag at all. > - If signaled and a wait on the async object is satisfied, the > requestor function should have performed the synchronous completion > sequence (either directly or via APC\_ASYNC\_IO) before the wait is > satisfied, unless `async->pending`. > - `direct_result`: from Wine codebase, `a flag if we're passing result > directly from request instead of APC`. > - Valid transition: 1 -> 0 (after set to 1 from `create_request_async`). > - Whenever async state is settled, the flag is set if all of the > following conditions are true: > - The async was created via create_request_async(), and > - async_set_unknown_status() has not been called, and > - The async handle has never been waited on (wait consumes the > flag), and > - Any of: > - async_handoff() has not been called, or > - From async_handoff(), the async is terminated or alerted synchronously. > - The flag has the following effect: > - For each async_terminate() call, it will send APC_ASYNC_IO if and > only if !direct_result. > - For each async handle wait satisfaction, it will call > async_set_result() if and only if direct_result. > We can observe the following from above: > - The flags are defined in terms of what the flags *do* rather than what > the value of the flags *mean*. It doesn't help that the transitions for > each flag are quite scattered and sparse. > - Their behaviors are dependent on the client-side async implementation, > so reasoning about them requires verifying how the requestor side behaves. > - Depending on other part of the async object state, one or more flags > may be left unobserved and thus ignored, effectivly becoming a "don't > care" bit. > This increases complexity and hinders maintenance in my opinion. In > particular, it's difficult to determine precisely *when* the flag is or > should be set or unset. > I'd propose that the constituent conditions of each flag be refactored > into separate state variables first. Paradoxically, it may even help > combine two or more binary flags into an enum as the semantics become > more simpler. Independent of above, I'm thinking about introducing a new `completed` flag and replacing `terminated` with `completed || alerted`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/499#note_31580

1 0

Re: [PATCH v3 0/1] MR499: server: Allow cancelling alerted asyncs.
by Jinoh Kang (＠iamahuman) April 29, 2023

April 29, 2023

On Tue Apr 18 19:25:54 2023 +0000, Zebediah Figura wrote: > > I don't think so. `async_terminate()` is currently no-op if the async > has already been terminated with a status other than `STATUS_ALERTED`, > and the patch maintains this invariant. > Right, that's the bit I failed to think through. > > It is possible to introduce a new boolean flag, but the flag cannot > replace `terminate_status`. `terminate_status` can be any arbitrary > value passed to the third parameter of `async_set_timeout`. > Ah, and I missed that that was a possibility, although, in my defense, > it's not stated in the patch subject either. I.e. this patch doesn't > just fix the race where an alerted async is canceled, it also fixes the > similar race where it times out. > So yes, I think the patch makes sense as is. Maybe a comment to yet > again overdocument asyncs wouldn't hurt? I easily see myself looking at > code like this and having to ask myself, "when exactly is > terminate_status set?" and the answer is "if (and only if?) the async is > canceled or times out while it's alerted", which is nice to have > documented and not have to puzzle through later. To reduce complexity, I've tried refactoring the state management. Currently, the async has the following two state flags that deal with the state of the client that originally requested the async: - `signaled`: indicates whether the async object is signaled; the client may wait on the async handle before returning from the corresponding wine system call. - Valid transition: 0 -> 1. - This flag is set if only if any of the following conditions are true: - Given that `!async->blocking`, - `async_wake_obj(async)` has been called when `async->unknown_status = 0`, or - `async_handoff(async, ...)` has been called with `get_error() != STATUS_ALERTED && (async->pending || !NT_ERROR(get_error()) && (get_error() != STATUS_PENDING || async->iosb->status != STATUS_PENDING)`. - (Regardless of `async->blocking`) `async_set_result(async, status, ...)` has been called with `async->terminated && (!async->alerted || status != STATUS_PENDING)`. - Note that this flag exhibits slightly different behavior depending on `async->blocking`. - If `async->blocking`, it tries to block the requestor until the async operation has completed. - If `!async->blocking`, it tries to signal as soon as the initial status is known, _unless_ the wait handle is being closed, in which case it doesn't bother to touch the flag at all. - If signaled and a wait on the async object is satisfied, the requestor function should have performed the synchronous completion sequence (either directly or via APC\_ASYNC\_IO) before the wait is satisfied, unless `async->pending`. - `direct_result`: from Wine codebase, `a flag if we're passing result directly from request instead of APC`. - Valid transition: 1 -> 0 (after set to 1 from `create_request_async`). - Whenever async state is settled, the flag is set if all of the following conditions are true: - The async was created via create_request_async(), and - async_set_unknown_status() has not been called, and - The async handle has never been waited on (wait consumes the flag), and - Any of: - async_handoff() has not been called, or - From async_handoff(), the async is terminated or alerted synchronously. - The flag has the following effect: - For each async_terminate() call, it will send APC_ASYNC_IO if and only if !direct_result. - For each async handle wait satisfaction, it will call async_set_result() if and only if direct_result. We can observe the following from above: - The flags are defined in terms of what the flags *do* rather than what the value of the flags *mean*. It doesn't help that the transitions for each flag are quite scattered and sparse. - Their behaviors are dependent on the client-side async implementation, so reasoning about them requires verifying how the requestor side behaves. - Depending on other part of the async object state, one or more flags may be left unobserved and thus ignored, effectivly becoming a "don't care" bit. This increases complexity and hinders maintenance in my opinion. In particular, it's difficult to determine precisely *when* the flag is or should be set or unset. I'd propose that the constituent conditions of each flag be refactored into separate state variables first. Paradoxically, it may even help combine two or more binary flags into an enum as the semantics become more simpler. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/499#note_31579

1 0

[PATCH 0/1] MR2330: xmllite/writer: implement prefix autogeneration
by David Kahurani (＠kahurani) April 29, 2023

April 29, 2023

Looks good to me but would love some opinion(s) Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2330

3 7

Re: [PATCH v14 0/4] MR2669: wine{alsa,coreaudio,oss,pulse}: Move AudioSessionControl into mmdevapi.
by Davide Beatrici (＠davidebeatrici) April 29, 2023

April 29, 2023

Davide Beatrici (@davidebeatrici) commented about dlls/mmdevapi/session.c: > +#include <winternl.h> > + > +#include <wine/debug.h> > +#include <wine/unixlib.h> > + > +#include "mmdevdrv.h" > +#include "unixlib.h" > + > +#define NULL_PTR_ERR MAKE_HRESULT(SEVERITY_ERROR, FACILITY_WIN32, RPC_X_NULL_REF_POINTER) > + > +WINE_DEFAULT_DEBUG_CHANNEL(mmdevapi); > + > +extern void sessions_lock(void) DECLSPEC_HIDDEN; > +extern void sessions_unlock(void) DECLSPEC_HIDDEN; > + > +extern const IAudioClient3Vtbl AudioClient3_Vtbl; Unused. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2669#note_31574

1 0