xblc: Migrate to SDL3 #2698

faha223 · 2026-01-24T19:15:05Z

Changed the usb-xblc driver to use the SDL3 audio backend

To test this: add an xblc to your controller using the following command in the qemu monitor:
device_add usb-xblc,port=1.3.2,id=xblc1

You can also remove the xblc from your controller using the following command in the qemu monitor:
device_del xblc1

This will add the virtual XBLC device to expansion slot A of the controller in port 1

gemini-code-assist · 2026-01-24T19:15:20Z

Summary of Changes

Hello @faha223, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly updates the USB Xbox Live Communicator (XBLC) driver by transitioning its audio processing from QEMU's legacy audio backend to SDL3. This migration modernizes the audio stack, leveraging SDL3's capabilities for more robust and potentially more performant audio input and output, and streamlines the device's audio stream management.

Highlights

SDL3 Migration: The USB Xbox Live Communicator (XBLC) driver has been migrated from QEMU's internal AudioBackend to use the SDL3 audio backend for both input and output streams.
Audio Stream Management: Refactored audio stream initialization and management, introducing a new XBLCStream struct to encapsulate SDL3 audio device and stream details, and a helper function xblc_audio_channel_init.
Callback Updates: The output_callback and input_callback functions have been rewritten to align with SDL3's audio stream callback signatures and utilize SDL_PutAudioStreamData and SDL_GetAudioStreamData respectively.
Underrun Prevention: Improved handling for audio underrun by ensuring that output packets are always filled, even with silence, to prevent audio glitches.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request successfully migrates the xblc driver to the SDL3 audio backend, refactoring the audio stream handling and updating the callbacks accordingly. The changes are generally good, but I've found a few critical issues related to potential null pointer dereferences when handling audio streams, which could lead to crashes. I've also identified a logic issue that could cause unnecessary re-initialization of audio streams and a minor performance issue in one of the new audio callbacks. Please address these points to ensure the stability and correctness of the driver.

gemini-code-assist · 2026-01-24T19:19:38Z

hw/xbox/xblc.c

+    channel->voice = SDL_OpenAudioDeviceStream(device_id,
+                                               &channel->spec, 
+                                               capture ? input_callback : output_callback,
+                                               (void*)s);
+
+    SDL_ResumeAudioStreamDevice(channel->voice);


SDL_OpenAudioDeviceStream can return NULL on failure, but its return value is not checked. This will cause a crash in the following call to SDL_ResumeAudioStreamDevice. Please add a NULL check to handle potential failures gracefully.

channel->voice = SDL_OpenAudioDeviceStream(device_id, &channel->spec, capture ? input_callback : output_callback, (void*)s); if (channel->voice == NULL) { DPRINTF("[xblc] Failed to open audio device stream: %s\n", SDL_GetError()); return; } SDL_ResumeAudioStreamDevice(channel->voice);

I implemented the suggested change

gemini-code-assist · 2026-01-24T19:19:38Z

hw/xbox/xblc.c

+    SDL_PauseAudioStreamDevice(s->in.voice);
+    SDL_PauseAudioStreamDevice(s->out.voice);
+    
    fifo8_destroy(&s->out.fifo);
    fifo8_destroy(&s->in.fifo);

-    AUD_close_out(s->audio_be, s->out.voice);
-    AUD_close_in(s->audio_be, s->in.voice);
+    SDL_DestroyAudioStream(s->in.voice);
+    s->in.voice = 0;
+    SDL_DestroyAudioStream(s->out.voice);
+    s->out.voice = 0;


The functions SDL_PauseAudioStreamDevice and SDL_DestroyAudioStream are called without checking if s->in.voice and s->out.voice are NULL. If a stream was not created successfully or has already been destroyed, this will result in a crash. Please add NULL checks before calling these functions.

if (s->in.voice) { SDL_PauseAudioStreamDevice(s->in.voice); } if (s->out.voice) { SDL_PauseAudioStreamDevice(s->out.voice); } fifo8_destroy(&s->in.fifo); fifo8_destroy(&s->out.fifo); if (s->in.voice) { SDL_DestroyAudioStream(s->in.voice); s->in.voice = 0; } if (s->out.voice) { SDL_DestroyAudioStream(s->out.voice); s->out.voice = 0; }

I implemented the suggested change

gemini-code-assist · 2026-01-24T19:19:38Z

hw/xbox/xblc.c

+    if(init_input_stream || init_output_stream) {
+        xblc_audio_channel_init(s, true);
+        xblc_audio_channel_init(s, false);
+    }


Both input and output audio streams are re-initialized if either one needs it. This can lead to unnecessary re-initialization and potential audio glitches on the stream that was already configured correctly. It's better to initialize them separately based on their respective flags.

if (init_input_stream) { xblc_audio_channel_init(s, true); } if (init_output_stream) { xblc_audio_channel_init(s, false); }

I implemented the suggested change

gemini-code-assist · 2026-01-24T19:19:38Z

hw/xbox/xblc.c

+    uint32_t total_bytes_read = 0;
+    uint32_t max_len = MIN(total_amount, fifo8_num_free(&s->in.fifo));
+    if (max_len > 0) {
+        memset(buffer, 0, max_len);


The call to memset to zero out the buffer before reading audio data with SDL_GetAudioStreamData is unnecessary, as SDL_GetAudioStreamData will overwrite the buffer's contents. This line can be removed to improve performance in this audio callback.

I implemented the suggested change

…improve stability. Removed an unnecessary memset in the input_callback.

Copilot

Pull request overview

This PR migrates the USB Xbox Live Communicator (XBLC) driver from the custom QEMU audio backend to SDL3's audio API. The migration replaces the previous AudioBackend/SWVoice system with SDL3's SDL_AudioStream and SDL_AudioDeviceStream APIs.

Changes:

Replaced custom audio backend structures with SDL3 audio streams
Refactored audio initialization to use SDL3 device streams
Updated audio callbacks to SDL3's callback signature and buffering approach
Simplified resource management with SDL3's lifecycle APIs

Copilot · 2026-01-25T18:01:25Z

hw/xbox/xblc.c

+    if(s->in.voice != 0)
+        SDL_LockAudioStream(s->in.voice);
+    if(s->out.voice != 0)
+        SDL_LockAudioStream(s->out.voice);
+
    fifo8_reset(&s->in.fifo);
    fifo8_reset(&s->out.fifo);
+
+    if(s->in.voice != 0)
+        SDL_UnlockAudioStream(s->in.voice);
+    if(s->out.voice != 0)
+        SDL_UnlockAudioStream(s->out.voice);


Missing braces around single-line if statement. According to QEMU coding style (docs/devel/style.rst:168), every indented statement must be braced even if the block contains just one statement.

Suggested change

if(s->in.voice != 0)

SDL_LockAudioStream(s->in.voice);

if(s->out.voice != 0)

SDL_LockAudioStream(s->out.voice);

fifo8_reset(&s->in.fifo);

fifo8_reset(&s->out.fifo);

if(s->in.voice != 0)

SDL_UnlockAudioStream(s->in.voice);

if(s->out.voice != 0)

SDL_UnlockAudioStream(s->out.voice);

if(s->in.voice != 0) {

SDL_LockAudioStream(s->in.voice);

}

if(s->out.voice != 0) {

SDL_LockAudioStream(s->out.voice);

}

fifo8_reset(&s->in.fifo);

fifo8_reset(&s->out.fifo);

if(s->in.voice != 0) {

SDL_UnlockAudioStream(s->in.voice);

}

if(s->out.voice != 0) {

SDL_UnlockAudioStream(s->out.voice);

}

Copilot · 2026-01-25T18:01:25Z

hw/xbox/xblc.c

+    if(s->in.voice == 0)
+        init_input_stream = true;
+    if(s->out.voice == 0)
+        init_output_stream = true;


Missing braces around single-line if statement. According to QEMU coding style (docs/devel/style.rst:168), every indented statement must be braced even if the block contains just one statement.

Suggested change

if(s->in.voice == 0)

init_input_stream = true;

if(s->out.voice == 0)

init_output_stream = true;

if (s->in.voice == 0) {

init_input_stream = true;

}

if (s->out.voice == 0) {

init_output_stream = true;

}

Copilot · 2026-01-25T18:01:26Z

hw/xbox/xblc.c

+    if(s->out.voice == 0)
+        init_output_stream = true;


Missing braces around single-line if statement. According to QEMU coding style (docs/devel/style.rst:168), every indented statement must be braced even if the block contains just one statement.

Copilot · 2026-01-25T18:01:26Z

hw/xbox/xblc.c

+
+    // Clear out the remainder of the input buffer
+    int bytes_read = 1;
+    while(bytes_read > 0 && total_bytes_read < total_amount) {


Missing space after while keyword. The QEMU coding style requires a space between control flow keywords and the opening parenthesis.

Suggested change

while(bytes_read > 0 && total_bytes_read < total_amount) {

while (bytes_read > 0 && total_bytes_read < total_amount) {

Copilot · 2026-01-25T18:01:26Z

hw/xbox/xblc.c

+static void input_callback(void *userdata, SDL_AudioStream *stream, int additional_amount, int total_amount)
+{
+    USBXBLCState *s = (USBXBLCState *)userdata;
+    static uint8_t buffer[XBLC_FIFO_SIZE];


The static buffer declared here makes the input_callback function non-reentrant, which could be problematic if SDL3 calls this callback from multiple threads or if multiple XBLC devices exist. Consider making this buffer part of the XBLCStream structure or using a per-device buffer allocated on the heap.

Suggested change

static uint8_t buffer[XBLC_FIFO_SIZE];

uint8_t buffer[XBLC_FIFO_SIZE];

I moved this buffer into the USBXBLCState structure because the output stream doesn't need it

Copilot · 2026-01-25T18:01:26Z

hw/xbox/xblc.c

+    if(s->in.voice != 0)
+        SDL_LockAudioStream(s->in.voice);
+    if(s->out.voice != 0)
+        SDL_LockAudioStream(s->out.voice);
+
    fifo8_reset(&s->in.fifo);
    fifo8_reset(&s->out.fifo);
+
+    if(s->in.voice != 0)
+        SDL_UnlockAudioStream(s->in.voice);
+    if(s->out.voice != 0)
+        SDL_UnlockAudioStream(s->out.voice);


Missing braces around single-line if statement. According to QEMU coding style (docs/devel/style.rst:168), every indented statement must be braced even if the block contains just one statement.

Suggested change

if(s->in.voice != 0)

SDL_LockAudioStream(s->in.voice);

if(s->out.voice != 0)

SDL_LockAudioStream(s->out.voice);

fifo8_reset(&s->in.fifo);

fifo8_reset(&s->out.fifo);

if(s->in.voice != 0)

SDL_UnlockAudioStream(s->in.voice);

if(s->out.voice != 0)

SDL_UnlockAudioStream(s->out.voice);

if (s->in.voice != 0) {

SDL_LockAudioStream(s->in.voice);

}

if (s->out.voice != 0) {

SDL_LockAudioStream(s->out.voice);

}

fifo8_reset(&s->in.fifo);

fifo8_reset(&s->out.fifo);

if (s->in.voice != 0) {

SDL_UnlockAudioStream(s->in.voice);

}

if (s->out.voice != 0) {

SDL_UnlockAudioStream(s->out.voice);

}

Copilot · 2026-01-25T18:01:27Z

hw/xbox/xblc.c

+        if(bytes_read > 0)
+            total_bytes_read += bytes_read;


Missing braces around single-line if statement. According to QEMU coding style (docs/devel/style.rst:168), every indented statement must be braced even if the block contains just one statement.

Suggested change

if(bytes_read > 0)

total_bytes_read += bytes_read;

if(bytes_read > 0) {

total_bytes_read += bytes_read;

}

Copilot · 2026-01-25T18:01:27Z

hw/xbox/xblc.c

+    if(channel->voice != 0) {
+        SDL_PauseAudioStreamDevice(channel->voice);


Missing braces around single-line if statement. According to QEMU coding style (docs/devel/style.rst:168), every indented statement must be braced even if the block contains just one statement.

Copilot · 2026-01-25T18:01:27Z

hw/xbox/xblc.c

-
-    s->out.voice = AUD_open_out(s->audio_be, s->out.voice, TYPE_USB_XBLC "-speaker",
-                                s, output_callback, &s->as);
+    if(s->sample_rate != sample_rate) {


Missing space after if keyword. The QEMU coding style requires a space between control flow keywords and the opening parenthesis, similar to other if statements in the codebase (e.g., hw/usb/bus.c:58, hw/usb/core.c:86).

Suggested change

if(s->sample_rate != sample_rate) {

if (s->sample_rate != sample_rate) {

Copilot · 2026-01-25T18:01:27Z

hw/xbox/xblc.c

+    if(s->in.voice != 0)
+        SDL_LockAudioStream(s->in.voice);
+    if(s->out.voice != 0)
+        SDL_LockAudioStream(s->out.voice);
+
    fifo8_reset(&s->in.fifo);
    fifo8_reset(&s->out.fifo);
+
+    if(s->in.voice != 0)
+        SDL_UnlockAudioStream(s->in.voice);
+    if(s->out.voice != 0)
+        SDL_UnlockAudioStream(s->out.voice);


Missing braces around single-line if statement. According to QEMU coding style (docs/devel/style.rst:168), every indented statement must be braced even if the block contains just one statement.

Suggested change

if(s->in.voice != 0)

SDL_LockAudioStream(s->in.voice);

if(s->out.voice != 0)

SDL_LockAudioStream(s->out.voice);

fifo8_reset(&s->in.fifo);

fifo8_reset(&s->out.fifo);

if(s->in.voice != 0)

SDL_UnlockAudioStream(s->in.voice);

if(s->out.voice != 0)

SDL_UnlockAudioStream(s->out.voice);

if (s->in.voice != 0) {

SDL_LockAudioStream(s->in.voice);

}

if (s->out.voice != 0) {

SDL_LockAudioStream(s->out.voice);

}

fifo8_reset(&s->in.fifo);

fifo8_reset(&s->out.fifo);

if (s->in.voice != 0) {

SDL_UnlockAudioStream(s->in.voice);

}

if (s->out.voice != 0) {

SDL_UnlockAudioStream(s->out.voice);

}

…ad in the USBBLCState structure. This will prevent multiple devices from interfering with each other if the input callback is called on multiple threads simultaneously Fixed some inconsistencies with the QEMU code style guidelines.

mborgerson · 2026-01-30T17:51:30Z

hw/xbox/xblc.c


    DPRINTF("[XBLC] Reset\n");
+
+    if (s->in.voice != 0) {


nit: Since this is a pointer, prefer checking against NULL instead

There were a lot of instances where that was being compared or assigned 0 instead of NULL. I went through and fixed them

mborgerson · 2026-01-30T19:04:55Z

hw/xbox/xblc.c

 * QEMU USB Xbox Live Communicator (XBLC) Device
 *
 * Copyright (c) 2022 Ryan Wendland
+ * Copyright (c) 2025 faha223


If you add your name here, prefer to use your real name

Changed to my real name

mborgerson · 2026-01-30T19:06:28Z

hw/xbox/xblc.c

-        processed = AUD_write(s->out.voice, (void *)data, max_len);
-        avail    -= processed;
-        if (processed < max_len) return;
+static void input_callback(void *userdata, SDL_AudioStream *stream, int additional_amount, int total_amount)


SDL3 enables us to simplify this code and drop these extra fifos and callbacks. Let's refactor this code to use the direct stream approach instead of callbacks

I've refactored the xblc to remove the fifo queues and callbacks, and I'm not directly calling SDL_PutAudioStreamData and SDL_GetAudioStreamData inside the usb_xblc_handle_data function. It appears that it still works. There's slightly less latency now when testing with N.U.D.E.

mborgerson · 2026-01-30T19:08:33Z

hw/xbox/xblc.c

+    // Clear out the remainder of the input buffer
+    int bytes_read = 1;
+    while (bytes_read > 0 && total_bytes_read < total_amount) {
+        bytes_read = SDL_GetAudioStreamData(stream, s->input_buffer, MIN(XBLC_FIFO_SIZE, total_amount - total_bytes_read));


If SDL_GetAudioStreamData is returning <0, something is wrong. We should not hammer on it here

If SDL_GetAudioStreamData returns <0 then it exits the loop. I'm not sure what else to do when that happens.

mborgerson · 2026-01-30T19:15:15Z

hw/xbox/xblc.c

+    if (s->sample_rate != sample_rate) {
+        init_input_stream = true;
+        init_output_stream = true;
+        s->sample_rate = sample_rate;


We shouldn't need to recreate the stream to change the sample rate. SDL3 offers an API for this SDL_SetAudioStreamFormat

I changed this function so that it uses SDL_SetAudioStreamFormat if the streams are not NULL instead of calling xblc_audio_channel_init

… NULL. Changed the xblc_audio_stream_init function to call SDL_SetAudioStreamFormat instead of calling xblc_audio_channel_init when the device is already initialized but the sample rate is being changed. Refactored the XBLCStream to get rid of the input_buffer and the fifo queues and callbacks and just call SDL_GetAudioStreamData and SDL_PutAudioStreamData directly in the usb_xblc_handle_data function

mborgerson · 2026-01-31T17:25:49Z

hw/xbox/xblc.c

+        while(to_process) {
+            chunk_len = MIN(p->iov.size, sizeof(s->out.packet));
+            usb_packet_copy(p, (void *)s->out.packet, chunk_len);
+            SDL_PutAudioStreamData(s->out.voice, s->out.packet, chunk_len);


Is it possible that s->out.voice is not initialized here? E.g. if xblc_audio_stream_init is never called

SDL_PutAudioStreamData may fail

Why do we need this intermediate buffer s->out.packet?

I'm not sure

That's true. I'm not sure what to do when it fails though

It turns out we don't. I can just call SDL_PutAudioStreamData using p->iov.iov->iov_base directly

I suppose if it fails I should log the error and assert(false);

mborgerson · 2026-01-31T17:26:29Z

hw/xbox/xblc.c

        while (to_process) {
-            const uint8_t *packet = fifo8_pop_bufptr(&s->in.fifo, to_process, &chunk_len);
-            usb_packet_copy(p, (void *)packet, chunk_len);
+            chunk_len = SDL_GetAudioStreamData(s->in.voice, s->in.packet, MIN(to_process, p->iov.size));


Is it possible that s->in.voice is not initialized here? E.g. if xblc_audio_stream_init is never called

to_process will always be <= p->iov.size

mborgerson · 2026-01-31T17:26:43Z

hw/xbox/xblc.c

-            const uint8_t *packet = fifo8_pop_bufptr(&s->in.fifo, to_process, &chunk_len);
-            usb_packet_copy(p, (void *)packet, chunk_len);
+            chunk_len = SDL_GetAudioStreamData(s->in.voice, s->in.packet, MIN(to_process, p->iov.size));
+            if(chunk_len < 0)


nit: Needs braces

mborgerson · 2026-01-31T17:29:05Z

hw/xbox/xblc.c

-        fifo8_push_all(&s->out.fifo, s->out.packet, to_process);
+
+        // Process the desired amount of data
+        to_process = p->iov.size;


Comment is redundant

hw/xbox/xblc.c

mborgerson · 2026-01-31T17:35:30Z

hw/xbox/xblc.c

+        // Process the desired amount of data
+        to_process = p->iov.size;
+
+        // In case the out packet is smallerthan the iov size, do it in chunks


Redundant comment

mborgerson · 2026-01-31T17:36:27Z

hw/xbox/xblc.c

+        to_process = p->iov.size;
+
+        // In case the out packet is smallerthan the iov size, do it in chunks
+        while(to_process) {


nit: Spaces. In this case feel free to just run clang-format on this file to normalize whitespace, clang-format config in root dir

Added some error checking Combined some if statements to reduce the code size. Removed some redundant comments Added return value checks to some functions and added some error logging and asserts Added a call to xblc_audio_stream_ini to usb_xbox_communicator_realize to ensure the audio streams are initialized from the moment the device is connected Added a call to SDL_ClearAudioStream at the end of xblc_audio_stream_init to ensure that the stream is empty after setting the sample rate. This resolves the latency problem I've been seeing in N.U.D.E. where it turns the mic on and off but the audio stream keeps filling up while the mic is off. Not sure if this applies to other games.

…f before the last commit

mborgerson · 2026-02-03T19:33:16Z

hw/xbox/xblc.c

-        } while (avail > 0 && processed >= XBLC_MAX_PACKET);
-        return;
+    if (s->in.voice != NULL) {
+        SDL_LockAudioStream(s->in.voice);


Why is it necessary to lock the audio streams?

According to the docs for SDL_ClearAudioStream:

It is safe to call this function from any thread.

Leftover from SDL2 and from when we were using callbacks. I've removed these locks

mborgerson · 2026-02-03T19:34:56Z

hw/xbox/xblc.c

+                                            SDL_AUDIO_DEVICE_DEFAULT_PLAYBACK;
+
+    if (channel->voice != NULL) {
+        SDL_PauseAudioStreamDevice(channel->voice);


Why are we pausing the stream before destruction? I think SDL_DestroyAudioStream should implicitly pause a stream

This was leftover from when we were using the callbacks and managing the queued data ourselves. This isn't necessary so I removed it.

mborgerson · 2026-02-03T19:36:07Z

hw/xbox/xblc.c

+        if (s->in.voice == NULL) {
+            DPRINTF("[XBLC] Tried to get data from the input audio tream but "
+                    "the audio stream is not initialized");
+            assert(false);


Why assert? xblc emulation should still work, even if we failed to bring up audio streams

mborgerson · 2026-02-03T19:36:19Z

hw/xbox/xblc.c

+
+        to_process = MIN(p->iov.size, available);
+
+        // Read data from the stream


Redundant comment

mborgerson · 2026-02-03T19:37:57Z

hw/xbox/xblc.c

+            assert(false);
+        }
+
+        int available = SDL_GetAudioStreamAvailable(s->in.voice);


Do we need to check if there are data available? What if we just attempt to directly SDL_GetAudioStreamData instead

We don't need to check. It's just useful info to have when debugging. I'll remove it

mborgerson · 2026-02-03T19:38:36Z

hw/xbox/xblc.c

+        if (s->out.voice == NULL) {
+            DPRINTF("[XBLC] Tried to put data into the speaker audio stream "
+                    "but the audio stream is not initialized");
+            assert(false);


Same comment as above. We should still be able to emulate an XBLC even if SDL3 streams aren't available

mborgerson · 2026-02-03T19:38:54Z

hw/xbox/xblc.c

+                                    p->iov.size)) {
+            DPRINTF("[XBLC] Error putting data into output stream: %s\n",
+                    SDL_GetError());
+            assert(false);


Same comment as above

mborgerson · 2026-02-03T19:42:03Z

hw/xbox/xblc.c

-        processed = AUD_write(s->out.voice, (void *)data, max_len);
-        avail    -= processed;
-        if (processed < max_len) return;
+    SDL_ClearAudioStream(s->in.voice);


Let's check that the stream != NULL before calling SDL_ClearAudioStream

~~Should we be clearing input here? See comment below~~ Actually clearing here probably makes sense to discard queued data that might increase latency

mborgerson · 2026-02-03T19:43:38Z

hw/xbox/xblc.c


-    s->in.voice = AUD_open_in(s->audio_be, s->in.voice, TYPE_USB_XBLC "-microphone",
-                                s, input_callback, &s->as);
+    SDL_ClearAudioStream(s->in.voice);


Should only clear a stream if it exists. If we initialized a new one, we probably don't need to clear it?

According to https://wiki.libsdl.org/SDL3/SDL_SetAudioStreamFormat it appears you do not need to clear streams when the format changes, so maybe we can just drop the clear

There were 2 reasons why I added this:

When calling SDL_SetAudiostreamFormat, any samples already in the queue will be in the previous format

This solves a problem I was seeing in N.U.D.E. where the queue was filling up with data while the mic was "off" (not being read, but still active). I noticed that every time the mic starts to be used a command is sent to set the sample rate, so I use that as a cue to clear the queue

When calling SDL_SetAudiostreamFormat, any samples already in the queue will be in the previous format

According to SDL_SetAudioStreamFormat docs:

Future calls to and SDL_GetAudioStreamAvailable and SDL_GetAudioStreamData will reflect the new format, and future calls to SDL_PutAudioStreamData must provide data in the new input formats.

So I don't think we need to worry about old format of samples--SDL should handle this for us.

his solves a problem I was seeing in N.U.D.E. where the queue was filling up with data while the mic was "off" (not being read, but still active)

This makes sense. But isn't this going to be done in usb_xblc_handle_reset?

mborgerson · 2026-02-03T21:03:38Z

hw/xbox/xblc.c

-    AUD_close_out(s->audio_be, s->out.voice);
-    AUD_close_in(s->audio_be, s->in.voice);
+    if (s->in.voice) {
+        SDL_PauseAudioStreamDevice(s->in.voice);


Same comment as above. Do we really need to pause before destroying a stream?

It appears that we don't, so I removed the call to SDL_PauseAudioStreamDevice

…oStream, and SDL_UnlockAudioStream, and SDL_GetAudioStreamAvailable. Removed a redundant comment Removed some asserts from usb_xblc_handle_data error conditions because the error conditions don't degrade the emulator. The errors are still being logged

…'t call SDL_ClearAudoStream on NULL

…stead, I added some code to usb_xblc_handle_data to call SDL_ClearAudioStream on the mic stream if the number of available bytes exceeds an amount of data equivalent to 100 ms at the current sample rate.

mborgerson · 2026-02-09T07:06:44Z

hw/xbox/xblc.c

-            usb_packet_copy(p, (void *)packet, chunk_len);
+            chunk_len = SDL_GetAudioStreamData(s->in.voice, s->in.packet,
+                                               MIN(sizeof(s->in.packet), to_process));
+            if (chunk_len < 0) {


If we break out here, should we be providing silence to the guest as before?

If SDL_GetAudioStreamData doesn't have enough data to provide, it will spin here and block emulation. Previous behavior was to pull as much as possible from the FIFO and provide silence for the rest of the packet. Blocking might be better to introduce some latency in order to avoid underruns, but totally blocking emulation here is not ideal. A simple solution to both underruns and blocking could be to just provide silence for a while until the stream has buffered at least a full packets worth of data plus some headroom on the input stream.

Yeah, I don't see any harm in filling the buffer with silence if we don't have enough data to fill the buffer. We definitely don't want to wait for new data if we can avoid it

mborgerson · 2026-02-09T07:18:39Z

hw/xbox/xblc.c

+    SDL_AudioDeviceID device_id;
+    SDL_AudioStream *voice;
+    SDL_AudioSpec spec;
+    uint8_t packet[XBLC_MAX_PACKET];


Do we need the packet field anymore? Looks like it's only used usb_xblc_handle_data USB_TOKEN_IN case, so we can probably replace with a local buffer there

We only need it for the USB_TOKEN_IN case, so it can be replaced with a local buffer. It's only 48 bytes

mborgerson · 2026-02-09T07:27:01Z

hw/xbox/xblc.c


-    fifo8_create(&s->in.fifo, XBLC_FIFO_SIZE);
-    fifo8_create(&s->out.fifo, XBLC_FIFO_SIZE);
+    // What sample rate does the physical XBLC default to?


Is this a TODO or FIXME or something else? If so, we should indicate as such. Any reason for picking the sample rate at index 1, or was this an arbitrary choice? Why not highest or lowest?

this was a FIXME, but I went looking and, according to the source code for Ryzee119's HAWK XBLC replacement, the XBLC appears to default to 16 KHz, so I changed it to 16KHz here, and added a comment explaining where I got that number.

mborgerson · 2026-02-09T07:31:22Z

hw/xbox/xblc.c

+                                               24000 };
+
+typedef struct XBLCStream {
+    SDL_AudioDeviceID device_id;


Is the device_id field needed? It is set, but never read, from xblc_audio_channel_init

It'll be needed for the UI changes, but those aren't in this PR so I'll remove it

…ta when there isn't enough data available to fill the request. Added code to fill the rest of the buffer with silence. Changed the default sample rate of the XBLC to 16 KHz, and updated the comment to point to where I got that number from. Removed the device_id and packet fields from the XBLCStream struct. Replaced the packet field with a local variable in the usb_xblc_handle_data function

xblc: Migrate to SDL3

f46bb38

gemini-code-assist bot reviewed Jan 24, 2026

View reviewed changes

Implemented some checks that were suggested by gemini-code-assist to …

23e179f

…improve stability. Removed an unnecessary memset in the input_callback.

mborgerson requested review from Ryzee119 and Copilot January 25, 2026 17:56

Copilot started reviewing on behalf of mborgerson January 25, 2026 17:57 View session

Copilot AI reviewed Jan 25, 2026

View reviewed changes

mborgerson reviewed Jan 30, 2026

View reviewed changes

faha223 added 2 commits January 31, 2026 01:28

Merge branch 'master' into xblc_sdl3

23aa1e4

mborgerson reviewed Jan 31, 2026

View reviewed changes

faha223 added 4 commits February 1, 2026 15:28

Merge branch 'master' into xblc_sdl3

23e9942

Fixed some things that I broke when I was cleaning up some debug stuf…

11b38ba

…f before the last commit

Merged the checks in usb_xbox_communicator_unrealize

93c248d

mborgerson reviewed Feb 3, 2026

View reviewed changes

faha223 added 4 commits February 5, 2026 17:31

Merge branch 'master' into xblc_sdl3

9ec1852

Added a check before clearing the input stream queue, to ensure I don…

0519fff

…'t call SDL_ClearAudoStream on NULL

Removed the SDL_ClearAudioStream call from xblc_audio_stream_init. In…

9ebf76f

…stead, I added some code to usb_xblc_handle_data to call SDL_ClearAudioStream on the mic stream if the number of available bytes exceeds an amount of data equivalent to 100 ms at the current sample rate.

mborgerson reviewed Feb 9, 2026

View reviewed changes

Merge branch 'master' into xblc_sdl3

ca7741c

	while(bytes_read > 0 && total_bytes_read < total_amount) {
	while (bytes_read > 0 && total_bytes_read < total_amount) {

	static uint8_t buffer[XBLC_FIFO_SIZE];
	uint8_t buffer[XBLC_FIFO_SIZE];

		if(channel->voice != 0) {
		SDL_PauseAudioStreamDevice(channel->voice);

	if(s->sample_rate != sample_rate) {
	if (s->sample_rate != sample_rate) {


		to_process = MIN(p->iov.size, available);

		// Read data from the stream

Uh oh!

xblc: Migrate to SDL3 #2698

Are you sure you want to change the base?

xblc: Migrate to SDL3 #2698

Uh oh!

Conversation

faha223 commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Jan 24, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

faha223 commented Jan 24, 2026 •

edited

Loading

faha223 Feb 1, 2026 •

edited

Loading

mborgerson Feb 3, 2026 •

edited

Loading