Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

digitalsignalperson
Copy link

Solves #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

  • rebase off dev branch.
  • line 31793 // pContext->jack.noAutoConnect = true; // TODO I don't know how to use custom context config, it crashes

…ure.channels unless they are set to 0, then use the number of channels in the default device
@mackron
Copy link
Owner

mackron commented May 24, 2024

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
Copy link
Author

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
Copy link
Author

digitalsignalperson commented May 24, 2024

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

  • ma_device_init
    • ma_context_init
      • ma_context_init__jack: pConfig here is context config. We don't have device config if we wanted to store requestd number of channels somewhere
        • ma_context_open_client__jack (temp dummy client)
    • ma_device_init_jack: here we use pDevice->playback->channels or (device config) pConfig->playback->channels. Oh maybe here they can be stored in pDevice->pContext so it can be available to ma_context_get_device_info__jack later
      • ma_context_open_client__jack
    • ma_device_post_init -> ma_device_get_info -> ma_context_get_device_info
      • ma_context_get_device_info__jack: here we need num channels but only have pContext. pDeviceID is null and pDeviceInfo is zero initialized.
        • ma_context_open_client__jack
  • ma_device_start
    • ma_device_start__jack: here we use pDevice->playback->channels or pConfig->playback->channels

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants