mackron / miniaudio

Audio playback and capture library written in C, in a single source file.
https://miniaud.io
Other
4.07k stars 361 forks source link

jack: add noAutoConnect option and allow arbitrary number of channels #852

Open digitalsignalperson opened 5 months ago

digitalsignalperson commented 5 months ago

Solves https://github.com/mackron/miniaudio/issues/851

I'm testing with this callback to play a sine wave over all channels

void data_callback(ma_device* pDevice, void* pOutput, const void* pInput, ma_uint32 frameCount) {
    (void) pInput; // unused
    float* pFramesOutF32 = (float*)pOutput;
    static size_t n = 0;
    float s;
    for (int i = 0; i < frameCount; i++) {
        s = sin((float)n / 48000.0f * 6.0f * 200.0f);
        n++;
        for (int j = 0; j < pDevice->playback.channels; j++) {
            *pFramesOutF32++ = s;
    }
    }
}

Default case:

    ma_device_config deviceConfig = ma_device_config_init(ma_device_type_playback);
    deviceConfig.playback.format = ma_format_f32;
    //deviceConfig.playback.channels = 2;
    deviceConfig.sampleRate = 48000.0f;
    deviceConfig.dataCallback = data_callback;
    if (ma_device_init(NULL, &deviceConfig, &state.device) != MA_SUCCESS) {
        printf("Failed to open playback device.\n");
    }
    ma_device_start(&state.device);

image

I think this is still wrong. I have 2 audio devices with 8 outputs and 2 outputs, and here miniaudio creates 10 outputs and connects them all. I'm not sure what the "default device" is defined as with jack. Also this is with pipewire-jack.

Setting .playback.channels=2

Un-commenting that one line results in: image

Setting .jack.noAutoconnect = true

Caveat that I couldn't figure out how to use ma_context_config without crashing, so on line 31793 I just force the value to true.

image

The miniaudio device has 2 channels and initially not connected to anything.

I can now patch them however I want image

Here's with 32 channels: image

TODO

I must not be using ma_context_config properly, or there is a bug.

On the master branch without this patch, this code will segfault in ma_malloc from ma_device__jack_buffer_size_callback.

    ma_device_config deviceConfig = ma_device_config_init(ma_device_type_playback);
    deviceConfig.playback.format = ma_format_f32;
    deviceConfig.playback.channels = 32;
    deviceConfig.sampleRate = 48000.0f;
    deviceConfig.dataCallback = data_callback;

    ma_context_config contextConfig = ma_context_config_init();
    // contextConfig.jack.noAutoConnect = true;
    ma_context context;
    ma_backend backends[] = { ma_backend_jack };
    ma_result result = ma_context_init(backends, 1, &contextConfig, &context);
    if (result != MA_SUCCESS) {
        printf("failed to init context");
    }
    if (ma_device_init(&context, &deviceConfig, &state.device) != MA_SUCCESS) {
        printf("Failed to open playback device.\n");
    }

If I try it on my patch here, strangely I see it worked until I uncomment contextConfig.jack.noAutoConnect = true;. Idk, something bad trashing the memory perhaps.

TODO

mackron commented 5 months ago

What was your intention for the jack.channelsCapture/Playback variables in the context? It's just that it's very unusual to have a channel count at the context level rather than the device level. Regardless, this code will need to change:

    if (pContext->backend == ma_backend_jack) {
        pContext->jack.channelsCapture = pConfig->capture.channels;
        pContext->jack.channelsPlayback = pConfig->playback.channels;
    }

Any backend-specific logic like that needs to be within the backend itself, not in the cross-platform section.

Just to clarify the rules. When you specify a channel count in the device config, the caller is specifying what they want. The backend (jack in this case), will use that as a hint. If it can exactly represent that channel count, it should do so, and if not, it should get as close as possible and then report via pPlayback/CaptureDescriptor the actual channel count. Then from that information miniaudio will do all the necessary conversion for you. From the perspective of the caller, they must always have the channel count they request in the device config, unless they pass in 0 in which case the backend should choose the "native" channel count.

digitalsignalperson commented 5 months ago

Oh about jack.channelsCapture/Playback, yes that was a bit of stop gap. I wasn't sure how to solve this but just wanted to get a proof of concept to see it working. Details:

The number of channels set in the ma_device_config struct is then stored in these locations:

pDevice->capture.channels            = pConfig->capture.channels;
pDevice->playback.channels           = pConfig->playback.channels;
descriptorPlayback.channels                 = pConfig->playback.channels;
descriptorCapture.channels                  = pConfig->capture.channels;

and while ma_device_get_info() is passed ma_device* pDevice in the first arg, when it calls ma_context_get_device_info(pDevice->pContext, type, pDevice->playback.pID, pDeviceInfo); which calls ma_context_get_device_info__jack(ma_context* pContext, ma_device_type deviceType, const ma_device_id* pDeviceID, ma_device_info* pDeviceInfo) it no longer has access to pDevice so doesn't know the requested number of channels. (But there's a good chance I'm missing some method to access it)

Since I saw pContext->jack.tryStartServer = pConfig->jack.tryStartServer; having a ma_device_config member being copied into pContext, I copied that as a way to have that info be avail to the jack implementation.

Oh and I think there was a reason I couldn't put pContext->jack.channelsCapture = pConfig->capture.channels; etc in ma_context_init__jack because ma_context_get_device_info__jack is called earlier? I'd have to take another look.

Also re: the channel count config rules, I do feel it wasn't easy to understand the different .channels floating around in this patch, so may need some more rewriting to be super clear as to how the jack implementation is getting/setting. number of channels. Like the code around using ppPortsCapture could use some pondering

digitalsignalperson commented 5 months ago

OK just stepping through the calls to jack backend to trace things out for a better understanding:

So does it make sense to have to store pContext->jack->channels{Playback,Capture}? Or else is there a way in ma_context_get_device_info__jack() to have the device pointer? The issue is the ma_context_get_device_info() function prototype does not pass a pDevice arg. But if this pDevice->pContext->callbacks.onDeviceGetInfo callback exists it is passed pDevice and that result used instead of calling ma_context_get_device_info(), so maybe that is where a ma_context_get_device_info__jack() with a matching signature should be. Otherwise a global change to the ma_context_get_device_info() function prototype to add an arg for pDevice?