Coreaudio_exclusive only work with audio-format=s16 on a 24bit external DAC

macdavis commented 7 years ago

mpv version and platform

MacOS 10.12.5 (16F73), MPV Git 7e889e5

Reproduction steps

Run with -no-config -audio-format=s16 -audio-exclusive=yes and -no-config -audio-format=s24 -audio-exclusive=yes

The specification of my DAC. It is 24 bit compatible. "[ 0.292][v][ao/coreaudio_exclusive] - 192000.0Hz 24bit mcpl [4][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 176400.0Hz 24bit mcpl [4][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 96000.0Hz 24bit mcpl [4][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 88200.0Hz 24bit mcpl [4][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 48000.0Hz 24bit mcpl [4][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 44100.0Hz 24bit mcpl [4][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 192000.0Hz 16bit mcpl [12][4bpp][1fbp][4bpf][2ch] int LE S packed (s16) [ 0.292][v][ao/coreaudio_exclusive] - 176400.0Hz 16bit mcpl [12][4bpp][1fbp][4bpf][2ch] int LE S packed (s16) [ 0.292][v][ao/coreaudio_exclusive] - 96000.0Hz 16bit mcpl [12][4bpp][1fbp][4bpf][2ch] int LE S packed (s16) [ 0.292][v][ao/coreaudio_exclusive] - 88200.0Hz 16bit mcpl [12][4bpp][1fbp][4bpf][2ch] int LE S packed (s16) [ 0.292][v][ao/coreaudio_exclusive] - 48000.0Hz 16bit mcpl [12][4bpp][1fbp][4bpf][2ch] int LE S packed (s16) [ 0.292][v][ao/coreaudio_exclusive] - 44100.0Hz 16bit mcpl [12][4bpp][1fbp][4bpf][2ch] int LE S packed (s16) [ 0.292][v][ao/coreaudio_exclusive] - 192000.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 176400.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 96000.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 88200.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 48000.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 44100.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 0.292][v][ao/coreaudio_exclusive] - 192000.0Hz 16bit mcpl [76][4bpp][1fbp][4bpf][2ch] int LE S packed (s16) [ 0.292][v][ao/coreaudio_exclusive] - 176400.0Hz 16bit mcpl [76][4bpp][1fbp][4bpf][2ch] int LE S packed (s16) [ 0.292][v][ao/coreaudio_exclusive] - 96000.0Hz 16bit mcpl [76][4bpp][1fbp][4bpf][2ch] int LE S packed (s16) [ 0.292][v][ao/coreaudio_exclusive] - 88200.0Hz 16bit mcpl [76][4bpp][1fbp][4bpf][2ch] int LE S packed (s16) [ 0.292][v][ao/coreaudio_exclusive] - 48000.0Hz 16bit mcpl [76][4bpp][1fbp][4bpf][2ch] int LE S packed (s16) [ 0.292][v][ao/coreaudio_exclusive] - 44100.0Hz 16bit mcpl [76][4bpp][1fbp][4bpf][2ch] int LE S packed (s16)"

Expected behavior

Both audio-format=s16 and audio-format=s24should work properly

Actual behavior

audio-format=s16 works fine. audio-format=s24 terminates.

Error message: "[ 1.009][v][ao/coreaudio_exclusive] virtual format 96000.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) [ 1.009][e][ao/coreaudio_exclusive] hardware format not supported [ 1.009][e][ao] Failed to initialize audio driver 'coreaudio' [ 1.009][e][cplayer] Could not open/initialize audio device -> no sound. [ 1.009][v][ad] Uninit audio decoder. [ 1.009][v][af] Removing filter lavrresample [ 1.010][i][cplayer] Audio: no audio [ 1.010][v][cplayer] playback restart complete [ 1.010][v][cplayer] EOF code: 1
[ 1.010][v][vd] Uninit video. [ 1.010][v][cplayer] finished playback, audio output initialization failed (reason 4)"

Log file

s16.log s24.log

Sample files

Sample files needed to reproduce this issue can be uploaded to https://0x0.st/ or similar sites. (Only needed if the issue cannot be reproduced without it.)

downthomas commented 7 years ago

From the logs, when you select -audio-format=s24, it looks like where it is erroring out in the code is in ao_coreaudio_exclusive.c: if (!ao->format || af_fmt_is_planar(ao->format)) { MP_ERR(ao, "hardware format not supported\n"); And it looks like the format is being set to 0 because the BitsPerChannel/BytesPerFrame aren't matching up. And that's happening with 24bit audio because in audio/format.c, af_fmt_to_bytes is returning 3 for AF_FORMAT_S24 instead of 4.

My initial response would be to change the return of af_fmt_to_bytes for AF_FORMAT_S24 to 4. But I don't know whether that might break things elsewhere.

So I don't know what the appropriate fix should be.

macdavis commented 7 years ago

Thanks, @downthomas . I did some test according to your suggestion, but it still failed. The only change I made was case AF_FORMAT_S24: , from return 3; to return 4; in audio/format.c.

After this change, I noted that although BytesPerFrame matched my device ([8bpp][1fbp][8bpf][2ch]), the requested bit depth became 32bit ([ao/coreaudio_exclusive] our format: 96000.0Hz 32bit mcpl [12][8bpp][1fbp][8bpf][2ch] int LE S packed (s24)). Now the Bit Depth can't match up.

The log file is here. http://sprunge.us/IVDL

Not sure whether the following information would be helpful.

In Audirvana Plus, if playing in Integer Mode, the debug information says, Device: 2ch Non-mixable linear PCM Interleaved 24bits little endian Signed Integer aligned low in 32 bit, 8 bytes per frame 96kHz
Apple Core Audio Format Specification says In the packed case, each 24 bit sample takes up 3 bytes in the file. In the unpacked case, the 24 bits are aligned high within the 4 byte field so that a parser can treat the value as if it were 32 bit integer with the lowest (or least significant) 8 bits all zero). https://developer.apple.com/library/content/documentation/MusicAudio/Reference/CAFSpec/CAF_spec/CAF_spec.html It seems changing from "3" to "4" resulting in unpacked audio. However, in the MPV's log, it still treats as packed.

ghost commented 7 years ago

That change will definitely break. CAF isn't the API. "24 bit" in 32 bit with data in high bytes is S32 in mpv. 24 bit support should probably somehow be redone, though it's all a pain. I'd argue AOs should always take 32 bit samples for 24 bit, and "reduce" the data themselves if needed.

macdavis commented 7 years ago

@wm4 Just wondering whether there is a way to change BytesPerFrame to ([8bpp][1fbp][8bpf][2ch]) while still keeping 24bit?

ghost commented 7 years ago

Not sure what you mean.

Anyway, does any of you want to change the code so that it accepts 32 bit samples instead of S24? I think I need to remove the S24 sample format from mpv anyway.

macdavis commented 7 years ago

Sorry about my English.

As downthomas pointed out, Coreaudio_exclusive doesn't work with 24 bit audio because the Bytes per packet, Frames per packet and Bytes per frame doesn't match that of my device. For 16 bit, since [4bpp][1fbp][4bpf] matches my device format, it works.

Currently, for 24 bit audio, mpv delivers [6bpp][1fbp][6bpf]. However, the device requires [8bpp][1fbp][8bpf]. The method downthomas suggested did change to [8bpp][1fbp][8bpf], but result in 32bit. I guess if [8bpp][1fbp][8bpf] and 24bit can be achieved at same time, perhaps it will work. That's why I raised that question.

I also found another reference, http://kaniini.dereferenced.org/2014/08/31/CoreAudio-sucks.html. It says,

CoreAudio takes the lower bits of data, not the higher bits of data. So if you were to implement 24-bit PCM support, you would pack the data into a series of 32-bit integers and tell CoreAudio to use 24 bits of each sample for PCM data.

downthomas commented 7 years ago

Well, since changing the return of af_fmt_to_bytes didn't fix the problem, that indicates there would still be a problem in the CoreAudio exclusive code recognizing the format. So maybe changing the CoreAudio code to recognize 24bit audio in 6 byte and 8 byte formats would allow it to work.

This change appeared to work for me. [ao/coreaudio_exclusive] setting stream physical format: 44100.0Hz 24bit mcpl [4][8bpp][1fbp][8bpf][2ch] int LE S (-) [ao/coreaudio_exclusive] virtual format 44100.0Hz 32bit mcpl [9][8bpp][1fbp][8bpf][2ch] float LE U packed (float)

Try using the function in the attached patch for audio/out/ao_coreaudio_utils.c instead:

ao_coreaudio_utils.txt

ghost commented 7 years ago

Well, since changing the return of af_fmt_to_bytes didn't fix the problem,

mpv's S24 is always 3 bytes, so the format mapping within ao_coreaudio is the problem. I probably never tested this - AFAIK coreaudio used to accept only float for a while?

downthomas commented 7 years ago

That's what CoreAudio looks like it is still using behind the scenes once it starts playing. It is a problem with the format mapping within ao_coreaudio. And thinking about it some more, I don't believe CoreAudio is ever going to offer a 24bit format with 6bpp/bpf. So maybe a better change would be to change ca_fill_asbd_raw to fill S24 with 8bpp/bpf instead. ao_coreaudio_utils.txt

ghost commented 7 years ago

I was under the impression that coreaudio does support 24 bit per samples, at least in theory (on the API level).

I'm not sure why that format would be a special case. If it's not supported, it wouldn't appear anywhere in this file at all. And it would mean either ca_fill_asbd_raw or ca_asbd_equals has a bug, because it uses CA's generic format description wrong.

downthomas commented 7 years ago

I believe your impression is correct. As for why it is a special case, I think I've found the answer. From the Core Audio Format Specification:

"Samples of 24 bits are commonly stored within PCM CAF files in either 3 bytes per sample (packed) or 4 bytes per sample (unpacked) formats. To conform to the CAF specification, you must support both storage methods."

So it looks like the root of the problem are the CoreAudio_utils code not supporting both packed and unpacked 24 bit samples. Packed S24 will be 6bpp, unpacked S24 will be 8bpp.

macdavis commented 7 years ago

@downthomas Tried both of your patches, still not working.

For patch mp_asbd.mBytesPerPacket with 3 byte,

[ao/coreaudio_exclusive] our format: 96000.0Hz 24bit mcpl [12][6bpp][1fbp][6bpf][2ch] int LE S packed (s24) [ao/coreaudio_exclusive] setting stream physical format: 96000.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) [ao/coreaudio_exclusive] virtual format 96000.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) [ao/coreaudio_exclusive] hardware format not supported

logfile: patch_1_mp_asbd.mBytesPerPacket_3.log

For patch mp_asbd.mBytesPerPacket with 4 byte,

[ao/coreaudio_exclusive] our format: 96000.0Hz 32bit mcpl [12][8bpp][1fbp][8bpf][2ch] int LE S packed (s24) [ao/coreaudio_exclusive] setting stream physical format: 96000.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) virtual format 96000.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-)

Unfortunately my device can't handle the physical format of 32 bit. In fact, most of the external DACs can't. But in Audirvana, I can see a virtual format of "2 ch Mixable linear PCM Interleaved 32 little endian Float 96kHz".

logfile: patch_1_mp_asbd.mBytesPerPacket_4.log

For patch S24_with_8bpp_bpf,

[ao/coreaudio_exclusive] our format: 96000.0Hz 24bit mcpl [12][8bpp][1fbp][8bpf][2ch] int LE S packed (s24) [ao/coreaudio_exclusive] setting stream physical format: 96000.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) [ao/coreaudio_exclusive] format in use before switching: 96000.0Hz 24bit mcpl [4][8bpp][1fbp][8bpf][2ch] int LE S (-) [ao/coreaudio_exclusive] virtual format 96000.0Hz 24bit mcpl [68][8bpp][1fbp][8bpf][2ch] int LE S (-) [ao/coreaudio_exclusive] hardware format not supported

Is it possible that the failure is due to the mismatch of the "number before [8bpp]"?

our format: 96000.0Hz 24bit mcpl [12] This [12] appears in my physical format of 16 bit

Logfile: patch_2_S24_with_8bpp_bpf.log

downthomas commented 7 years ago

Yeah, I think the continued failure is due to the AudioStreamBasicDescription mFormatFlags (the numbers in square brackets) being improperly set. There were three problems I saw:

Setting the Packed flag for all formats.
Not comparing flags accurately when comparing ASBDs.
Using the 3 byte Packed S24 format with physical formats. I have only seen 4 byte physical formats with CoreAudio.

But at the end of it, the 'hardware format not supported' message is still because the format wasn't being set.

This patch should set the appropriate mFormatFlags, compare those flags accurately so the appropriate format should be recognized, and set the right bpp/bpf. Try it out and see if works. ao_coreaudio_utils_diff.txt

macdavis commented 7 years ago

@downthomas Still not successful. Now the error message becomes Unsupported unaligned read of 4096 bytes

There is no sound and it keeps popping up that error message until I terminate it manually.

The log file: http://sprunge.us/SagF.

I also upload the altered ao_coreaudio_utils.c in case that I accidentally messed up something during the alteration. ao_coreaudio_utils_altered.c

BTW, I saw this in /audio/out/ao_coreaudio_exclusive.c

// we expect the callback to read full frames, which are aligned accordingly
    if (pseudo_frames * ao->sstride != requested) {
        MP_ERR(ao, "Unsupported unaligned read of %d bytes.\n", requested);
        return kAudioHardwareUnspecifiedError;
    }

ghost commented 7 years ago

That 24 bit special case in the patch still makes no sense, and it probably the reason for the unaligned read message (indicates that mpv is working with 3 byte samples while CA wants 4 bytes).

macdavis commented 7 years ago

@downthomas I studied the behaviour of both external DAC and built-in output device on Audirvana Plus. Guessing perhaps when playing 24-bit audio, changing the virtual format to 32-bit might solve the problem. Or can virtual format be different from physical format?

Built-in output device

According to Audirvana Plus, the built-in output device has 4 virtual formats, which are all 32-bit float. Also, the built-in output device isn’t compatible with the Integer Mode.

Virtual Format of Built-in audio device: 2 ch Mixable linear PCM Interleaved 32 little endian Float @ 44.1, 48, 88.2, 96 kHz

In mpv with -audio-format=s24, the virtual format is 32-bit (as it is the only option) virtual format 96000.0Hz 32bit mcpl [9][8bpp][1fbp][8bpf][2ch] float LE U packed (float) is chosen, resulting in the final format to be 96000Hz stereo 2ch float.

Log file: http://sprunge.us/OAai

External 24-bit DAC

Audirvana Plus shows there are 18 virtual formats:

Virtual Format of external 24-bit DAC: 2 ch Mixable linear PCM Interleaved 32 little endian Float @ 44.1, 48, 88.2, 96, 176.4, 192kHz 2 ch Non-mixable linear PCM Interleaved 24 little endian Signed Integer aligned low in 32bit @ 44.1, 48, 88.2, 96, 176.4, 192kHz
2 ch Non-mixable linear PCM Interleaved 16 little endian Signed Integer @ 44.1, 48, 88.2, 96, 176.4, 192kHz

When Integer Mode is off, it shows

Currently playing in standard 32bit float mode Device: 2ch Mixable linear PCM Interleaved 32bits little endian Float, 8 bytes per frame 96kHz

My hardware can’t handle actual 32-bit audio, but now the music is still playable. So, I’m guessing perhaps it is due to 32-bit Float virtual format.

When integer mode is on, it shows

Currently playing in Integer Mode: Device: 2ch Non-mixable linear PCM Interleaved 24bits little endian Signed Integer aligned low in 32 bit, 8 bytes per frame 96kHz

I believe your pervious patch is a foundation of 24-bit Integer Mode. I once read, Integer Mode means an integer physical format. At least, Audirvana applies unpacked 24-bit to achieve Integer Mode.

In mpv, I never saw 32-bit virtual format with my external DAC.

Sorry if I made any mistakes. It is purely deduction from my observations.

ghost commented 7 years ago

The s24 mpv sample format just stopped existing (for other reasons). But whatever the underlying problem is, it probably still exists.

macdavis commented 7 years ago

@wm4 The problem does still exist. CoreAudio can deal with 32-bit while CoreAudio Exclusive can't.

downthomas commented 7 years ago

Sorry for the radio silence, I looked at the problem for a while, dug into understanding Core Audio, then got busy, and didn't come back with what I was thinking.

There are two problems in the code that I've identified so far:

The CoreAudio utils code improperly setting up the AudioStreamBasicDescription.
The af_format returning 3 bytes for 24 bit audio.

Try using the ao_coreaudio_utils_diff.txt I linked to earlier, along with changing, in format.c, the return of af_fmt_to_bytes for AF_FORMAT_S24 to 4, and see if that works. Unfortunately, I don't have a USB Sound output to test this out on my own.

The first problem in the code is what prevents mpv from recognizing 24 bit output. Technically, mBytesPerPacket/mBytesPerFrame is supposed to be number_of_channels sizeof(ContainerDataType), not number_of_channels mBitsPerChannel. For hardware purposes, CoreAudio only supports unpacked 24 bit audio, so the container size will be 32 bit (4 bytes).

The second problem is harder to deal with conceptually, because I'm not sure if AF_FORMAT_24 is supported to represent the data size or the container size. More later.

macdavis commented 7 years ago

It's alright, @downthomas. I am and will always be grateful for your help.

I modified "ao_coreaudio_utils_diff.txt" patch from:

  if (asbd->mBitsPerChannel == 24) {
    asbd->mBytesPerPacket = asbd->mBytesPerFrame =
        asbd->mFramesPerPacket * channels_per_buffer *
        (asbd->mBitsPerChannel / 6);
 } else {

to

if (asbd->mBitsPerChannel == 32) {
    if (af_fmt_is_float(mp_format)) {
        asbd->mBytesPerPacket = asbd->mBytesPerFrame =
        asbd->mFramesPerPacket * channels_per_buffer *
        (asbd->mBitsPerChannel / 8);
       } else if (af_fmt_is_planar(mp_format)) {
                 asbd->mFormatFlags |= kAudioFormatFlagIsNonInterleaved;
                 asbd->mBytesPerPacket = asbd->mBytesPerFrame =
                 asbd->mFramesPerPacket * channels_per_buffer *
                 (asbd->mBitsPerChannel / 8);
        } else if (BYTE_ORDER == BIG_ENDIAN) {
                 asbd->mFormatFlags |= kAudioFormatFlagIsBigEndian;
                 asbd->mBytesPerPacket = asbd->mBytesPerFrame =
                 asbd->mFramesPerPacket * channels_per_buffer *
                 (asbd->mBitsPerChannel / 8);         
    } else {asbd->mBitsPerChannel = 24;
            asbd->mBytesPerPacket = asbd->mBytesPerFrame =
        asbd->mFramesPerPacket * channels_per_buffer *
        (asbd->mBitsPerChannel / 6); }
} else {

ao_coreaudio_utils.c: https://0x0.st/GbQ.c

Now it works on my external DAC as well as on the build-in audio device. However, there is one regression. 24-bit WAV/AIFF can only be played in Exclusive Mode. To play in Shared Mode, we need to specify --audio-format=float

downthomas commented 7 years ago

Well, I'm glad that it's working, because that shows it can work.

The ASBD (Audio Stream Basic Description) is still wrong however, and the code which selects which ASBD format to output to is still wrong. That's a third problem I identified: if you specify a specific format, like s24, the code is written to still prefer s32 or float if it is available. Now, it just thinks the s24 is an s32, so it is using that.

That's why to get it to play in Shared Mode, you have to specify float, because the ao_coreaudio_utils is used by both shared and exclusive. And in shared mode, audio is mixable, so mpv should feed it to CoreAudio in 32-bit float, because even if it didn't, CoreAudio would convert it (losslessly) to 32-bit float itself.

macdavis commented 7 years ago

Thanks for your detailed explanation.

My observation is if we don't use if (asbd->mBitsPerChannel == 32) {, everything works fine except that the audio breaks in Exclusive Mode with a DAC of 24-bit physical format. If we use if (asbd->mBitsPerChannel == 32) {, everything also works fine except that playing 24-bit LPCM breaks in Shared Mode.

It seems the problem is AO-dependent. As you said, in shared mode, audio must be true 32-bit for mixing. I tested, --audio-format=float, floatp, s32p, all works, as long as it's not s32. It makes sense, because I specifically defined "s32" to be "24-bit in 32-bit container". Maybe we could temporarily bypass this issue by first determining the current AO, that is

If current AO is coreaudio_exclusive, then use the patch (if (asbd->mBitsPerChannel == 32) {)

I'm not sure whether it is feasible in terms of implementation.

ghost commented 7 years ago

How can I reproduce any of these problems?

macdavis commented 7 years ago

@wm4

To reproduce playing 24-bit LPCM in Shared Mode problem, you need a). This ao_coreaudio_utils.c: https://0x0.st/GbQ.c b). This 24-bit LPCM sample: https://0x0.st/GMn.aiff Set AO to coreaudio, play this sample with Mac's build-in audio device. There is no error message in the terminal, but the sound is all high frequency noise.
To reproduce the 24-bit DAC and CoreAudio Exclusive Mode compatibility issue, you need a standalone 24-bit DAC. In my case, I can see "Unmixable 2 channel 24 Bit Signed Integer Aligned Low in 32 Bits" in both virtual format and physical format in this software, Apple's HALLab Can be downloaded here

Additionally, With the help of Apple's HALLab and Audirvana Plus, I came to the following conclusion. a.) Integer Mode is also called "Nonmixable Integer Stream" mode https://www.audioasylum.com/cgi/vt.mpl?f=pcaudio&m=126457

b.) With a proper hardware, mpv's CoreAudio Exclusive Mode is actually "Exclusive + Integer" Mode (Apple's HALLab shows "Unmixable 2 channel 24 Bit Signed Integer Aligned Low in 32 Bits" in both virtual format and physical format when playing with mpv as well as with Audirvana Plus)

c.) If Apple's HALLab shows virtual format is "Mixable 2 channel 32 Bit Float Point" while physical format is "Mixable 2 channel 24 Bit Signed Integer Aligned Low in 32 Bits", that means "Exclusive + Float" Mode. As explained by downthomas, CoreAudio would convert it (losslessly) to 32-bit float itself in order to be mixable. It' been confirmed by both mpv playing with Mac's build-in audio device (my Mac's build-in audio device has no unmixable format hence can't achieve Integer Mode, mpv log shows coreaudio_exclusive receives float point) and playing with Audirvana Plus (Integer Mode: OFF, log shows play in standard float point mode).

d.) Apple provides a kAudioFormatFlagIsNonMixable flag (https://developer.apple.com/documentation/coreaudio/1572097-audiostreambasicdescription_flag/kaudioformatflagisnonmixable?language=objc) I added (flags & kAudioFormatFlagIsNonMixable) ? " Non-mixable" : " Mixable", to this part of ao_coreaudio_utils.c. https://0x0.st/GMh.c

MP_VERBOSE(ao, "%s %7.1fHz %" PRIu32 "bit %s " "[%" PRIu32 "][%" PRIu32 "bpp][%" PRIu32 "fbp]" "[%" PRIu32 "bpf][%" PRIu32 "ch] " "%s %s %s%s%s%s (%s)\n", description, asbd->mSampleRate, asbd->mBitsPerChannel, format, asbd->mFormatFlags, asbd->mBytesPerPacket, asbd->mFramesPerPacket, asbd->mBytesPerFrame, asbd->mChannelsPerFrame, (flags & kAudioFormatFlagIsFloat) ? "float" : "int", (flags & kAudioFormatFlagIsBigEndian) ? "BE" : "LE", (flags & kAudioFormatFlagIsSignedInteger) ? "S" : "U", (flags & kAudioFormatFlagIsPacked) ? " packed" : "", (flags & kAudioFormatFlagIsAlignedHigh) ? " aligned" : "", (flags & kAudioFormatFlagIsNonInterleaved) ? " P" : "", mpfmt ? af_fmt_to_str(mpfmt) : "-"); }

Now the log can show whether the physical format is Mixable or not.

ghost commented 7 years ago

Ah, I doubt I have hardware for this.

What's the minimum required OSX version? I only have Sierra.

Is the entire API exclusive mode uses even still needed?

macdavis commented 7 years ago

I'm on Sierra as well. Integer Mode was taken out from Lion and brought back since Mavericks. So only Lion and Mountain Lion don't have Integer Mode. Unfortunately, I have no idea about any API changes.

I basically understand why this issue happens. In fact, even in Exclusive Mode, virtual format and physical format can be different. If they are different, both streams are mixable for format conversion (typically 32-bit virtual format to 24-bit physical format, which is also how it’s done in Shared Mode). In mpv’s Exclusive Mode, nonmixable format has higher preference, which means mpv prefers virtual and physical format having the same format, if hardware allows. Although my DAC has 32-bit virtual format, mpv still chooses 24-bit because of the physical format. Thus, problem raises when 24-bit virtual format can’t match 32bit "our format" (Audio Stream Basic Description). In CoreAudio’s definition, 24-bit is "Signed Integer Aligned Low in 32 Bits", which is "s32". That’s why I have to do some trick on this format, forcing "s32" to be "24-bit in a 32-bit container" to match my DAC's physical format.

In Shared Mode, Audio format must be "true 32-bit" for mixing. However, my patch specifically defines "s32" to be "24-bit". So, when playing "s32" in Shared Mode, the byte doesn't match up.

Maybe we could temporarily bypass this issue by first determining the current AO, that is

If current AO is coreaudio_exclusive, then use the patch (if (asbd->mBitsPerChannel == 32) {)

Alternatively, we can get rid of the Integer Mode (Personally, I prefer keeping Integer Mode). If "our format" (Audio Stream Basic Description) is 32-bit, set virtual format to be 32-bit, and let CoreAudio do the conversion.

Akemi commented 5 years ago

why? was this issue solved?

macdavis commented 5 years ago

Nah, I just open a new issue, including some thoughts and updates. Please see, https://github.com/mpv-player/mpv/issues/6750

macdavis commented 5 years ago

@downthomas You're right, The ASBD was indeed incorrect. mpv's format is actually 24 Bit aligned high in 32 Bit. If we don't specify kAudioFormatFlagIsAlignedHigh, Core Audio will take it as aligned low. The following code now works in both shared and exclusive mode.

if (af_fmt_is_planar(mp_format)) {
    asbd->mFormatFlags |= kAudioFormatFlagIsNonInterleaved;
    channels_per_buffer = 1;
}

if (af_fmt_is_float(mp_format)) {
    asbd->mFormatFlags |= kAudioFormatFlagIsFloat;
} else if (!af_fmt_is_unsigned(mp_format)) {
    asbd->mFormatFlags |= kAudioFormatFlagIsSignedInteger;
    if (asbd->mBitsPerChannel == 32){
      (asbd->mBitsPerChannel = 24);
      asbd->mFormatFlags |= kAudioFormatFlagIsAlignedHigh;
    }else{
      asbd->mFormatFlags |= kAudioFormatFlagIsPacked;
    }
}

if (BYTE_ORDER == BIG_ENDIAN)
    asbd->mFormatFlags |= kAudioFormatFlagIsBigEndian;

asbd->mFramesPerPacket = 1;

if (asbd->mBitsPerChannel ==24){
      asbd->mBytesPerPacket = asbd->mBytesPerFrame = 8;
}else{
asbd->mBytesPerPacket = asbd->mBytesPerFrame =
    asbd->mFramesPerPacket * channels_per_buffer *
    (asbd->mBitsPerChannel / 8);
}

Another issue I am worry about is the high/low alignment, that is our format being aligned high while the hardware format being aligned low. In exclusive mode where no format conversion is performed, I am worried about losing fidelity.

mpv has already constructed a function called ao_convert_inplace(struct ao_convert_fmt *fmt, void **data, int num_samples); to convert aligned high stream to aligned low, and this function is used in "ao_alsa" and "ao_wasapi". I don't understand how to use this function, could you please provide any insight?

badgerkin commented 5 years ago

From my reading, coreaudio_exclusive assigns render_cb_compressed as the callback function to provide the audio stream. My USB DACs offer

[ao/coreaudio_exclusive] - lpcm 24-bit 48.0KHz [76][6bpp][1fbp][6bpf][2ch] Packed Signed Integer Unmixable Aligned Low (-)

as a physical format. I'm not sure if the requisite byte shepherding can be accomplished by CoreAudio, or if that has to be handled client side via a call by ao_read_data_converted or similar.

macdavis commented 5 years ago

@badgerkin Your device is packed 24 Bit, which is different from mine. Does my patch work on your device? If not, we might need to make you a special case, too.

I'm not sure if the requisite byte shepherding can be accomplished by CoreAudio either, but worthy trying. CoreAudio is smarter than we though. Sometimes, it's just we setting too many hurdles in ca_asbd_equals.

badgerkin commented 5 years ago

Your patch gets coreaudio_exclusive to work in mixable float mode. When playing a 24/48 sample the console logs:

[ao/coreaudio_exclusive] Turn off Integer Mode as MSB padding is not implemented.
[ao/coreaudio_exclusive] setting stream physical format:lpcm 24-bit 48.0KHz [12][6bpp][1fbp][6bpf][2ch] Packed Signed Integer Mixable Aligned Low (-)
[ao/coreaudio_exclusive] format in use before switching:lpcm 24-bit 48.0KHz [12][6bpp][1fbp][6bpf][2ch] Packed Signed Integer Mixable Aligned Low (-)
[ao/coreaudio_exclusive] actual format in use:lpcm 24-bit 48.0KHz [12][6bpp][1fbp][6bpf][2ch] Packed Signed Integer Mixable Aligned Low (-)
...
[ao/coreaudio_exclusive] Virtual format: lpcm 32-bit 48.0KHz [9][8bpp][1fbp][8bpf][2ch] FloatPacked Mixable Aligned Low (float)

macdavis commented 5 years ago

Can you try this file? Previously, I set Integer Mode off if the physical format's highest bit is 24.

Edit: It still probably gonna fail. Things need to change (and maybe in different combinations):

return ~~(a->mFormatFlags & flags) == (b->mFormatFlags & flags) &&~~ a->mBitsPerChannel == b->mBitsPerChannel && ca_normalize_formatid(a->mFormatID) == ca_normalize_formatid(b->mFormatID) && Probably ~~(spdif || a->mBytesPerPacket == b->mBytesPerPacket) &&~~ (spdif || a->mChannelsPerFrame == b->mChannelsPerFrame) && a->mSampleRate == b->mSampleRate; in ca_asbd_equals

and maybe also MP_ERR(ao, "Unsupported unaligned read of %d bytes.\n", requested); ~~return kAudioHardwareUnspecifiedError;~~ in render_cb_compressed.

REMINDER, wrong alignment can cause very sharp noise. Make sure you reduce the volume before testing.

OR, a simpler solution, just use the original mpv git files and do the following changes in ca_asbd_equals/ao_coreaudio_utils.c. int flags = ~~kAudioFormatFlagIsPacked |~~ kAudioFormatFlagIsFloat | ~~a->mBitsPerChannel == b->mBitsPerChannel &&~~ Probably ~~(spdif || a->mBytesPerPacket == b->mBytesPerPacket) &&~~.

macdavis commented 5 years ago

@badgerkin Hi, can you try this patch? As suggested by you and @wm4 , I changed from ao_read_data to ao_read_data_converted.It makes a difference on my side, just turn out that I don't need to do the conversion. Can you test on your device?

badgerkin commented 5 years ago

It works, thank you!

I had to set sstride=6 at the top of render_cb_compressed() in addition to your patch, otherwise coreaudio doesn't get all the frames it expects, and plays at 0.75x normal speed/ buzzes.

macdavis commented 5 years ago

I'm glad it's working and thanks for your feedback. I tried to combine packed and unpacked cases, but lack of devices for testing. Could you please test this patch? If not working, could you please upload a full log?

badgerkin commented 5 years ago

This patch fails with:

[ao/coreaudio_exclusive] hardware format not supported
[ao] Failed to initialize audio driver 'coreaudio_exclusive'
[cplayer] Could not open/initialize audio device -> no sound.

Full log here.

macdavis commented 5 years ago

I think I made a mistake here. if (!integer_mode_unpacked_24_bit_device_hack(ao) && (p->stream_asbd.mFormatFlags & kAudioFormatFlagIsNonMixable) && (asbd.mBitsPerChannel == 32)) { new_format = ca_asbd_to_mp_format_integer_mode_hack(&p->stream_asbd); MP_VERBOSE(ao, "Hacking %u-bit stream on 24-bit device in Integer Mode\n", asbd.mBitsPerChannel); }else if (!integer_mode_packed_24_bit_device_hack(ao) && (p->stream_asbd.mFormatFlags & kAudioFormatFlagIsNonMixable) && (asbd.mBitsPerChannel == 32)){ new_format = ca_asbd_to_mp_format_integer_mode_packed_24_device_hack(&p->stream_asbd); MP_VERBOSE(ao, "Hacking %u-bit stream on packed 24-bit device in Integer Mode\n", asbd.mBitsPerChannel); }else{ new_format = ca_asbd_to_mp_format(&p->stream_asbd); }

This should fix it. Can you give a go?

badgerkin commented 5 years ago

That works. Alternatively, you could check for (max_mBytesPerPacket == 8) in integer_mode_unpacked_24_bit_device_hack() to avoid the false positive (or rather, false negative).

macdavis commented 5 years ago

You're right. Thanks for reminding.

macdavis commented 5 years ago

@badgerkin Hi, sorry to trouble you again. Could you please help to test the following code?

the latest commit - https://github.com/macdavis/mpv/commit/5f69382c6393adeeca921c5de2c95982f3c3e2af ~~(https://github.com/macdavis/mpv/commit/325aedfd196329a8b01d0178ac49eaf220a3e6c0)~~
If the above commit breaks 24 bit integer mode, could you please try this commit (https://github.com/macdavis/mpv/commit/325aedfd196329a8b01d0178ac49eaf220a3e6c0) (https://github.com/macdavis/mpv/commit/42dafa7edbfa425ee0d89b67332223e3bc3887c9)?

Basically, I did my best to fix all the bugs I introduced and make the code as concise as I can. If there is no problem on your device, I will open a PR.

If possible, please upload a full log as last time and focus on,

24 bit integer mode (integer mode should be on by default)
Turn off integer mode by coreaudio-integer-mode = no
Set audio I/O buffer size by coreaudio-buffer-size = (any integer from 0 to 24576)
Set IOCycle Usage by coreaudio-IOCycle-Usage = (float from 0 to 1)
Set audio power saving mode by coreaudio-power-saving = yes. This should not work in integer mode.

mpv-player / mpv