Fixes for Android sound driver. #2457
Conversation
Corrected a stray reference to float size, which should be int16_t, and may have caused a buffer overflow in some circumstances. Corrected a CodeQL multiplication overflow warning.
|
|
||
| memcpy ( audioData, outBuffer.data(), count * sizeof ( int16_t ) ); | ||
| memcpy ( intData, outBuffer.data(), sizeof ( int16_t ) * count ); |
There was a problem hiding this comment.
Note that by putting sizeof(int16_t) first in the expression, it should correctly avoid overflow without count having to be of type std::size_t. Similarly a few lines further down.
|
Tried installing this on my Moto G9 Power. 3.8.2rc1 installed OK. This one failed with a rather generic "Something went wrong" (joy of Android). I'll see if the previous build worked. |
|
Nope, basically I have to uninstall and reinstall on Android. But it's nothing related to this. |
|
OK, installed and I can connect and listen, very nicely, to a server in Germany (with apologies for forgetting to turn on "Mute Myself"). They could hear me, I could hear them. (They could hear me more than they or I wanted, in fact...) I don't have Stereo 3.5mm + Mono 1/4" to Headset 2.5mm adaptor or I'd have tried that (just to say hello :) ). |
Yep, tracked in #1760. |
|
Great, I'll try it on my Android phone tomorrow and then un-draft it. |
|
Another idea before I forget to write it down... It would be nice to have if the Android audio system could check whether it's only got the internal mic / audio out connected. If so, it should turn on "Mute Myself" and prevent it being turned off. (Maybe have a different banner - so we'd have "unmuted", "self-muted" and "hard muted".) |
|
@pljones - good point, but it will get lost here, once this is closed. Could you reference the above comment in a separate issue? |
|
Loaded the artifact on my phone (as before, needed to uninstall previous version first), and it appears to run as well as the previous version. So I think this is ready now. |
|
Ok. Merged it. |
The CodeQL entries under Security seem to persist, and show as either Fixed or Re-appeared, until their entries are deleted, I assume. There also seems to be some erraticness in whether they are flagged or not. I have observed messages saying "Fixed in commit xxx" and "Re-appeared in commit yyy", but a comparison of those commits shows the code in question has not changed. ??? I've also found some more similar entries in my own fork that don't appear under the main repo, but looking at the code suggests they are valid. I'll audit them over the next few days and raise another PR for them. |
|
Yep, it seems erratic almost to the point of worthlessness - but not quite. It is labelled "beta"... |
|
@pljones I've re written the changelog line a bit here too. |
Short description of changes
Corrected a stray reference to float size, which should be int16_t,
and may have caused a buffer overflow in some circumstances.
Corrected a CodeQL multiplication overflow warning.
CHANGELOG: Android: Improve sound driver to fix CodeQL warnings
Context: Fixes an issue?
CodeQL warning and incorrect data size calculation found from code inspection.
Does this change need documentation? What needs to be documented and how?
No
Status of this Pull Request
Hopefully ready, but awaiting CI and local test
What is missing until this pull request can be merged?
Check the CI
Checklist