Emulator Issues #4608: ACL queue tends to be overfilled (over 10 packets) in some situations, causing occasionnal packet loss (Wiimote disconnections, forgotten extensions, etc) -- issue introduced with r7394's functionality changes - Emulator - Dolphin bug tracker

Actions

Copy link

Emulator Issues #4608

closed

ACL queue tends to be overfilled (over 10 packets) in some situations, causing occasionnal packet loss (Wiimote disconnections, forgotten extensions, etc) -- issue introduced with r7394's functionality changes

Added by DimitriPilot3 over 13 years ago.

Status:

Fixed

Priority:

Normal

Assignee:

Category:

Controls

% Done:

Operating system:

N/A

Issue type:

Bug

Milestone:

Regression:

Relates to usability:

Yes

Relates to performance:

Easy:

Relates to maintainability:

Regression start:

Fixed in:

Description

I started investigating this issue (the first one) after finding this forum post: http://forums.dolphin-emulator.com/showthread.php?tid=16198&pid=153328#pid153328
Has this issue eventually been fixed in a later commit? I doubt so, judging by the apparent behaviour of example II (which is harder to reproduce after r7394 because micro-de-connections became more fatal; try it with 3 Wiimotes, while making sure to properly reconnect bugged-out Wiimotes by changing the Input Source between "Emulated" and "None")...

What steps will reproduce the problem?

Example I¶

Start up Dolphin (preferably r7272 or r7393, or any revision between these two - any later revision will make this issue harder to reproduce due to another issue);
Set up all four Wiimotes (emulated or real - shouldn't make a difference), and enable them;
Run a Wii game (such as SSBB (id RSBP for me) or NSMB Wii (id SMNP for me));
Do some controller input...

Example II¶

is the same step as in example I;
Set up (and enable) all four Wiimotes so they all have a Nunchuk attachment;
Run Super Smash Bros. Brawl (RSBP);
Go to a character select screen that shows four players (Group->Brawl);
Disconnect all four Wiimotes (ALT+F5, ALT+F6, ALT+F7 and ALT+F8);
After that, quickly reconnect all Wiimotes by holding ALT and pressing F5 then F6 then F7 then F8.
Observe the way in which the Wiimotes are being recognized and then "upgraded" to Nunchuk + Wiimote combos...

What is the expected output? What do you see instead?
##Example I
I expect output from all 4 Wiimotes to occur (somewhat) immediately after polling my gamepad. Instead, the delay between controller input and Wiimote output appears to increase.
And no, I am not talking about the lag created by Wiimote Speaker output; I am talking about a delay that seems independent from that, as the issue can occur at the beginning of emulation if all 4 Wiimotes are enabled.

##Example II
I expect the same behaviour as in r7271: fast recognition of Wiimote connections by the game, then a delay, then fast "upgrades" to Nunchuk/Wiimote combos, without any micro-disconnections (no temporary connection losses).
Since r7272, however, things have slowed down in a way that sometimes causes micro-disconnections, and sometimes causes a few "downgrades" followed with "upgrades". This also causes the symptoms of example I (delayed input/output) to be noticeable.

Dolphin version with the problem? Other Dolphin version without the
problem?
Both these problems are reproducible using Mamario's x86 builds of r7272 and r7393. These don't seem to occur when using Mamario's x86 build of r7271.

32-bit or 64-bit and any other build parameters?
To narrow down the culprit to a limited set of changes, I "updated" my repository to r7272, reverted changes to some files from r7272, and then recompiled Dolphin.
I brought back the "Wiimote Speaker" option that manipulated "IsBusyStrm_" in three different ways, only to see the two different issues (the Wiimote Speaker lag and the input/output delay).
When I reverted r7272's changes to WII_IPC_HLE_Device_usb (.h and .cpp) in my local repository, both these problems seem to have disappeared, at the cost of no Wiimote Speaker input.

OS version and versions of tools/libraries used?
Windows 7 Ultimate x86

Please provide any additional information below.
Well...
I am not sure where exactly (in WII_IPC_HLE_Device_usb) the problem may be (no ACL buffer overflow AFAIK), but I'd think that if the "culprit" is not going to be fixed (for 3.0?), this might be another piece of code that "Disable Wiimote Speaker" option should be toggling. Just a guess. Anyway, feel free to comment.

Related issues 2 (0 open — 2 closed)

Actions

Copy link

Updated by skidau over 13 years ago

DimitriPilot3, I have sent you a gmail with some tests I'd like you to run.

Actions

Copy link

Updated by DimitriPilot3 over 13 years ago

Well, I am not too curious about the correct timings that prevent disconnects, at least not now... Right now, I'm more interested in finding and understanding the changes that are causing these problems.

I've had a look at the WII_IPC_HLE_Device_usb changes in r7394 which replaced the std::queue class with a new ACLPool class. At first, the behaviours looked the same to me. But what I just noticed is that r7394 has restricted the ACL queue to a maximum of 10 packets (instead of "infinity" with std::queue).

So, theoretically, what would happen if ACLPool::Store() is called too often, as it might be the case since r7272? Then m_write_ptr may overlap m_read_ptr, which is bad. Whenever this happens, it'd look like the pool/queue is empty (because m_write_ptr == m_read_ptr) when it isn't, causing all old packets to be dropped forever (and overwritten by later Store() calls)... That's the only thing that can possibly explain the new Wiimote disconnection problems, introduced by r7394, described in issue 4353, and merely mentioned in this issue.

Is the limitation of the ACL pool/queue to 10 packets accurate to the real hardware? Is the behaviour of the ACL overflow/overrun accurate too? This behaviour seems off to me... (Shuffle? :p)

Anyway, if that limitation is really needed, then we should make sure that nothing in Dolphin's code (more specifically r7272, so let's compare r7271 and r7272?) makes the pool/queue grow too much in a way that makes the latency increase...

Actions

Copy link

Updated by Anonymous over 13 years ago

static const int m_acl_pkt_size = 339;
static const int m_acl_pkts_num = 10;
Is required code, because of the hardware we are emulating and how nintendo's software stack deals with it.

Also the frequency at which the device is updated similar to real hardware.

Here are some key things to look at:
http://code.google.com/p/dolphin-emu/source/browse/trunk/Source/Core/Core/Src/IPC_HLE/WII_IPC_HLE_Device_usb.cpp#498
http://code.google.com/p/dolphin-emu/source/browse/trunk/Source/Core/Core/Src/IPC_HLE/WII_IPC_HLE_Device_usb.cpp#1772

The problem is not the size of the queue growing too quickly and loosing packets, it's that dolphin is too slow (for various reasons) to complete the round-trip initiated by Wiimote::Update(wiimote_to_update);

There are two things keeping the queue length in check:

Nintendo's bluetooth stack (they use a modified WIDCOMM stack) checks statistics about the current pending and completed packets in order to manage the 10-packet buffer.
There is a check at http://code.google.com/p/dolphin-emu/source/browse/trunk/Source/Core/Core/Src/IPC_HLE/WII_IPC_HLE_Device_usb.cpp?spec=svn7571&r=7571#354 which would alert you if this happened. (Note, the comment from Ayuanx above this function is kinda out of date)

Actions

Copy link

Updated by Anonymous over 13 years ago

Also the frequency at which the device is updated is similar to real hardware.*

Actions

Copy link

Updated by Anonymous over 13 years ago

Add this to the list of things to look at http://code.google.com/p/dolphin-emu/source/browse/trunk/Source/Core/Core/Src/IPC_HLE/WII_IPC_HLE_Device_usb.cpp#1450

Actions

Copy link

Updated by DimitriPilot3 over 13 years ago

Category set to logic

Well, would you like to have a look at the patch I attached here? This should fix one half of this issue (especially if "Disable Wiimote Speaker" is checked). The other half is likely timing-related -- perhaps a missing "sleep" somewhere in WiimoteEmu code?

The change to WII_IPC_HLE_Device_usb.h exposes the issue I was talking about in my previous post (m_write_ptr overlapping m_read_ptr; as it turns out, that other "ACL buffer overflow" problem in the existing code isn't reported), and "fixes" it, by dropping the last packet instead of letting the pointers overlap which would make all queued packets unreachable.

The change to WII_IPC_HLE_Device_usb.cpp lowers the frequency of Wiimote::Update's to the "native" frequency if the "Disable Wiimote Speaker" option is ticked (150Hz/~6.66ms as noted in the comment above, instead of the actual 200Hz/5ms as required for Wiimote Speaker output).

Previous testing with 150 Hz (or less) showed that this resulted in quite a lot less "ACL queue is full" warnings than with the too-fast-for-Dolphin-to-handle 200 Hz. These warnings are still existent though, especially during simultaneous connections. Of course Wiimote Speaker output no longer works with this lowered frequency, so it's only used when the feature is disabled.

I have made more testings with this patch that may yield interesting results: http://pastebin.com/38Wnt0vf
Notice how using real Wiimotes instead of emulated Wiimotes decreases the amount of "ACL queue is full" warnings that occur when connections are being established? I'd guess it's because the connection phase for emulated Wiimotes (by that I especially mean the exchange of ACL packets for connection/configuration) is currently not as accurately timed as real Wiimotes, meaning that such ACL packets for emulated Wiimotes are being exchanged quicker than with real Wiimotes...

Any objections to this? I wanted to post this accurate-ish patch for 3.0, but I was too late, whatever...

Actions

Copy link

Updated by Anonymous over 13 years ago

Hmm, well that is troubling albeit good to know.

Actions

Copy link

Updated by hatarumoroboshi over 13 years ago

Wouldn't it be better to commit this patch in order to accurately test it and see if it fixes DKCR (HLE) Wiimote lag as well?

Actions

Copy link

Updated by stephen.gutknecht over 13 years ago

The patch seems to focus on the exposed issue... "r7394 has restricted the ACL queue to a maximum of 10 packets (instead of "infinity" with std::queue)"

I'm sympathetic to the developers that testing this is difficult... Dolphin really is stressed with 4 players on most hardware... even games like Wii Sports Tennis are doing the ball-hitting sound.. having 4 players all waving around their racket [which can be done even if only on has the ball] is a good stresser ;)

Actions

Copy link

#10

Updated by Anonymous over 13 years ago

Seriously, did you even read my response? That MUST be a static value of 10.

However, it seems like DP3 did find another issue, I just haven't had time to check into it. (Maybe DP3 has it covered :p)

Actions

Copy link

#11

Updated by stephen.gutknecht over 13 years ago

Seriously, yes? No need to be insulting you jerk.

Actions

Copy link

#12

Updated by Anonymous over 13 years ago

It's insulting to me if I post the exact reasons why something is the way it is, and then someone completely ignores them...

Actions

Copy link

#13

Updated by stephen.gutknecht over 13 years ago

I'm mostly kidding on the insult... Simply: I'm pro improved-logging to debug the problems ;)

Actions

Copy link

#14

Updated by DimitriPilot3 over 13 years ago

Category deleted (~~logic~~)

I have committed a variation of this patch in r7675. It should fix like half or a third of this issue, and it should be okay...

Not sure if and when I'm going to look after the remaining problems (which, to me, would be mostly timing-related; as in, things may need to be done more separately and/or in a different order) related to this issue -- time will likely tell that...

Actions

Copy link

#15

Updated by DimitriPilot3 over 13 years ago

...A bit shorter, perhaps?

Actions

Copy link

#16

Updated by glanabr over 13 years ago

I think this issue is "responsible" for the problem many users (including me) are having of not being able to connect 4 wiimotes. Would love to make tests if necessary, i got 4 real wiimotes and 2 nunchucks, i7-930, gtx260, windows7 ultimate 64 bits.

Actions

Copy link

#17