From: Marc-André Lureau <[email protected]> The audio_pcm_info structure stored three fields (bits, is_signed, is_float) that were always derived from the AudioFormat enum. This redundancy meant the same information was represented twice, with no type-level guarantee that they stayed in sync.
Replace these fields with a single AudioFormat field, and add helper functions to extract the derived properties when needed: - audio_format_bits() - audio_format_is_signed() - audio_format_is_float() This improves type safety by making AudioFormat the single source of truth, eliminating the possibility of inconsistent state between the format enum and its derived boolean/integer representations. Signed-off-by: Marc-André Lureau <[email protected]> Reviewed-by: Akihiko Odaki <[email protected]> Reviewed-by: Mark Cave-Ayland <[email protected]> --- audio/audio_int.h | 4 +- audio/audio_template.h | 12 +-- include/qemu/audio.h | 49 +++++++++ audio/audio-mixeng-be.c | 218 ++++++++++++---------------------------- audio/dbusaudio.c | 12 +-- audio/coreaudio.m | 2 +- 6 files changed, 126 insertions(+), 171 deletions(-) diff --git a/audio/audio_int.h b/audio/audio_int.h index 5334c4baad2..dd5f2220d75 100644 --- a/audio/audio_int.h +++ b/audio/audio_int.h @@ -45,9 +45,7 @@ struct audio_callback { }; struct audio_pcm_info { - int bits; - bool is_signed; - bool is_float; + AudioFormat af; int freq; int nchannels; int bytes_per_frame; diff --git a/audio/audio_template.h b/audio/audio_template.h index 08d60422589..3da91a4782c 100644 --- a/audio/audio_template.h +++ b/audio/audio_template.h @@ -173,7 +173,7 @@ static int glue (audio_pcm_sw_init_, TYPE) ( sw->empty = true; #endif - if (sw->info.is_float) { + if (audio_format_is_float(hw->info.af)) { #ifdef DAC sw->conv = mixeng_conv_float[sw->info.nchannels == 2] [sw->info.swap_endianness]; @@ -188,9 +188,9 @@ static int glue (audio_pcm_sw_init_, TYPE) ( sw->clip = mixeng_clip #endif [sw->info.nchannels == 2] - [sw->info.is_signed] + [audio_format_is_signed(hw->info.af)] [sw->info.swap_endianness] - [audio_bits_to_index(sw->info.bits)]; + [audio_format_to_index(hw->info.af)]; } sw->name = g_strdup (name); @@ -300,7 +300,7 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioMixengBackend *s, goto err1; } - if (hw->info.is_float) { + if (audio_format_is_float(hw->info.af)) { #ifdef DAC hw->clip = mixeng_clip_float[hw->info.nchannels == 2] [hw->info.swap_endianness]; @@ -315,9 +315,9 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioMixengBackend *s, hw->conv = mixeng_conv #endif [hw->info.nchannels == 2] - [hw->info.is_signed] + [audio_format_is_signed(hw->info.af)] [hw->info.swap_endianness] - [audio_bits_to_index(hw->info.bits)]; + [audio_format_to_index(hw->info.af)]; } glue(audio_pcm_hw_alloc_resources_, TYPE)(hw); diff --git a/include/qemu/audio.h b/include/qemu/audio.h index b6b6ee9b560..9e4566d60aa 100644 --- a/include/qemu/audio.h +++ b/include/qemu/audio.h @@ -185,6 +185,55 @@ bool audio_be_set_dbus_server(AudioBackend *be, const char *audio_application_name(void); +static inline int audio_format_bits(AudioFormat fmt) +{ + switch (fmt) { + case AUDIO_FORMAT_S8: + case AUDIO_FORMAT_U8: + return 8; + + case AUDIO_FORMAT_S16: + case AUDIO_FORMAT_U16: + return 16; + + case AUDIO_FORMAT_F32: + case AUDIO_FORMAT_S32: + case AUDIO_FORMAT_U32: + return 32; + + case AUDIO_FORMAT__MAX: + break; + } + + g_assert_not_reached(); +} + +static inline bool audio_format_is_float(AudioFormat fmt) +{ + return fmt == AUDIO_FORMAT_F32; +} + +static inline bool audio_format_is_signed(AudioFormat fmt) +{ + switch (fmt) { + case AUDIO_FORMAT_S8: + case AUDIO_FORMAT_S16: + case AUDIO_FORMAT_S32: + case AUDIO_FORMAT_F32: + return true; + + case AUDIO_FORMAT_U8: + case AUDIO_FORMAT_U16: + case AUDIO_FORMAT_U32: + return false; + + case AUDIO_FORMAT__MAX: + break; + } + + g_assert_not_reached(); +} + #define DEFINE_AUDIO_PROPERTIES(_s, _f) \ DEFINE_PROP_AUDIODEV("audiodev", _s, _f) diff --git a/audio/audio-mixeng-be.c b/audio/audio-mixeng-be.c index d02cfe3c509..931d5b206f2 100644 --- a/audio/audio-mixeng-be.c +++ b/audio/audio-mixeng-be.c @@ -62,23 +62,28 @@ int audio_bug (const char *funcname, int cond) return cond; } -static inline int audio_bits_to_index (int bits) +/* + * Convert audio format to mixeng_clip index. Used by audio_pcm_sw_init_ and + * audio_mixeng_backend_add_capture() + */ +static int audio_format_to_index(AudioFormat af) { - switch (bits) { - case 8: + switch (af) { + case AUDIO_FORMAT_U8: + case AUDIO_FORMAT_S8: return 0; - - case 16: + case AUDIO_FORMAT_U16: + case AUDIO_FORMAT_S16: return 1; - - case 32: + case AUDIO_FORMAT_U32: + case AUDIO_FORMAT_S32: return 2; - - default: - audio_bug ("bits_to_index", 1); - AUD_log (NULL, "invalid bits %d\n", bits); - return 0; + case AUDIO_FORMAT_F32: + case AUDIO_FORMAT__MAX: + break; } + + g_assert_not_reached(); } void AUD_vlog (const char *cap, const char *fmt, va_list ap) @@ -172,141 +177,68 @@ static int audio_validate_settings (const struct audsettings *as) static int audio_pcm_info_eq (struct audio_pcm_info *info, const struct audsettings *as) { - int bits = 8; - bool is_signed = false, is_float = false; - - switch (as->fmt) { - case AUDIO_FORMAT_S8: - is_signed = true; - /* fall through */ - case AUDIO_FORMAT_U8: - break; - - case AUDIO_FORMAT_S16: - is_signed = true; - /* fall through */ - case AUDIO_FORMAT_U16: - bits = 16; - break; - - case AUDIO_FORMAT_F32: - is_float = true; - /* fall through */ - case AUDIO_FORMAT_S32: - is_signed = true; - /* fall through */ - case AUDIO_FORMAT_U32: - bits = 32; - break; - - default: - abort(); - } - return info->freq == as->freq + return info->af == as->fmt + && info->freq == as->freq && info->nchannels == as->nchannels - && info->is_signed == is_signed - && info->is_float == is_float - && info->bits == bits && info->swap_endianness == (as->endianness != HOST_BIG_ENDIAN); } void audio_pcm_init_info (struct audio_pcm_info *info, const struct audsettings *as) { - int bits = 8, mul; - bool is_signed = false, is_float = false; - - switch (as->fmt) { - case AUDIO_FORMAT_S8: - is_signed = true; - /* fall through */ - case AUDIO_FORMAT_U8: - mul = 1; - break; - - case AUDIO_FORMAT_S16: - is_signed = true; - /* fall through */ - case AUDIO_FORMAT_U16: - bits = 16; - mul = 2; - break; - - case AUDIO_FORMAT_F32: - is_float = true; - /* fall through */ - case AUDIO_FORMAT_S32: - is_signed = true; - /* fall through */ - case AUDIO_FORMAT_U32: - bits = 32; - mul = 4; - break; - - default: - abort(); - } - + info->af = as->fmt; info->freq = as->freq; - info->bits = bits; - info->is_signed = is_signed; - info->is_float = is_float; info->nchannels = as->nchannels; - info->bytes_per_frame = as->nchannels * mul; + info->bytes_per_frame = as->nchannels * audio_format_bits(as->fmt) / 8; info->bytes_per_second = info->freq * info->bytes_per_frame; info->swap_endianness = (as->endianness != HOST_BIG_ENDIAN); } -void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len) +void audio_pcm_info_clear_buf(struct audio_pcm_info *info, void *buf, int len) { if (!len) { return; } - if (info->is_signed || info->is_float) { - memset(buf, 0x00, len * info->bytes_per_frame); - } else { - switch (info->bits) { - case 8: - memset(buf, 0x80, len * info->bytes_per_frame); - break; - - case 16: - { - int i; - uint16_t *p = buf; - short s = INT16_MAX; - - if (info->swap_endianness) { - s = bswap16 (s); - } - - for (i = 0; i < len * info->nchannels; i++) { - p[i] = s; - } - } - break; + switch (info->af) { + case AUDIO_FORMAT_U8: + memset(buf, 0x80, len * info->bytes_per_frame); + break; + case AUDIO_FORMAT_U16: { + int i; + uint16_t *p = buf; + short s = INT16_MAX; - case 32: - { - int i; - uint32_t *p = buf; - int32_t s = INT32_MAX; + if (info->swap_endianness) { + s = bswap16(s); + } - if (info->swap_endianness) { - s = bswap32 (s); - } + for (i = 0; i < len * info->nchannels; i++) { + p[i] = s; + } + break; + } + case AUDIO_FORMAT_U32: { + int i; + uint32_t *p = buf; + int32_t s = INT32_MAX; - for (i = 0; i < len * info->nchannels; i++) { - p[i] = s; - } - } - break; + if (info->swap_endianness) { + s = bswap32(s); + } - default: - AUD_log (NULL, "audio_pcm_info_clear_buf: invalid bits %d\n", - info->bits); - break; + for (i = 0; i < len * info->nchannels; i++) { + p[i] = s; } + break; + } + case AUDIO_FORMAT_S8: + case AUDIO_FORMAT_S16: + case AUDIO_FORMAT_S32: + case AUDIO_FORMAT_F32: + memset(buf, 0x00, len * info->bytes_per_frame); + break; + case AUDIO_FORMAT__MAX: + g_assert_not_reached(); } } @@ -719,8 +651,8 @@ static size_t audio_pcm_sw_write(SWVoiceOut *sw, void *buf, size_t buf_len) #ifdef DEBUG_AUDIO static void audio_pcm_print_info (const char *cap, struct audio_pcm_info *info) { - dolog("%s: bits %d, sign %d, float %d, freq %d, nchan %d\n", - cap, info->bits, info->is_signed, info->is_float, info->freq, + dolog("%s: %s, freq %d, nchan %d\n", + cap, AudioFormat_str(info->af), info->freq, info->nchannels); } #endif @@ -1759,15 +1691,15 @@ static CaptureVoiceOut *audio_mixeng_backend_add_capture( cap->buf = g_malloc0_n(hw->mix_buf.size, hw->info.bytes_per_frame); - if (hw->info.is_float) { + if (audio_format_is_float(hw->info.af)) { hw->clip = mixeng_clip_float[hw->info.nchannels == 2] [hw->info.swap_endianness]; } else { hw->clip = mixeng_clip [hw->info.nchannels == 2] - [hw->info.is_signed] + [audio_format_is_signed(hw->info.af)] [hw->info.swap_endianness] - [audio_bits_to_index(hw->info.bits)]; + [audio_format_to_index(hw->info.af)]; } QLIST_INSERT_HEAD (&s->cap_head, cap, entries); @@ -1869,29 +1801,6 @@ audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo) }; } -int audioformat_bytes_per_sample(AudioFormat fmt) -{ - switch (fmt) { - case AUDIO_FORMAT_U8: - case AUDIO_FORMAT_S8: - return 1; - - case AUDIO_FORMAT_U16: - case AUDIO_FORMAT_S16: - return 2; - - case AUDIO_FORMAT_U32: - case AUDIO_FORMAT_S32: - case AUDIO_FORMAT_F32: - return 4; - - case AUDIO_FORMAT__MAX: - ; - } - abort(); -} - - /* frames = freq * usec / 1e6 */ int audio_buffer_frames(AudiodevPerDirectionOptions *pdo, audsettings *as, int def_usecs) @@ -1914,8 +1823,7 @@ int audio_buffer_samples(AudiodevPerDirectionOptions *pdo, int audio_buffer_bytes(AudiodevPerDirectionOptions *pdo, audsettings *as, int def_usecs) { - return audio_buffer_samples(pdo, as, def_usecs) * - audioformat_bytes_per_sample(as->fmt); + return audio_buffer_samples(pdo, as, def_usecs) * audio_format_bits(as->fmt) / 8; } void audio_rate_start(RateCtl *rate) diff --git a/audio/dbusaudio.c b/audio/dbusaudio.c index 5f65c043c16..b212b3d6022 100644 --- a/audio/dbusaudio.c +++ b/audio/dbusaudio.c @@ -147,9 +147,9 @@ dbus_init_out_listener(QemuDBusDisplay1AudioOutListener *listener, qemu_dbus_display1_audio_out_listener_call_init( listener, (uintptr_t)hw, - hw->info.bits, - hw->info.is_signed, - hw->info.is_float, + audio_format_bits(hw->info.af), + audio_format_is_signed(hw->info.af), + audio_format_is_float(hw->info.af), hw->info.freq, hw->info.nchannels, hw->info.bytes_per_frame, @@ -273,9 +273,9 @@ dbus_init_in_listener(QemuDBusDisplay1AudioInListener *listener, HWVoiceIn *hw) qemu_dbus_display1_audio_in_listener_call_init( listener, (uintptr_t)hw, - hw->info.bits, - hw->info.is_signed, - hw->info.is_float, + audio_format_bits(hw->info.af), + audio_format_is_signed(hw->info.af), + audio_format_is_float(hw->info.af), hw->info.freq, hw->info.nchannels, hw->info.bytes_per_frame, diff --git a/audio/coreaudio.m b/audio/coreaudio.m index 0c0e8583c89..1f4b3d9d467 100644 --- a/audio/coreaudio.m +++ b/audio/coreaudio.m @@ -359,7 +359,7 @@ static OSStatus init_out_device(coreaudioVoiceOut *core) AudioValueRange frameRange; AudioStreamBasicDescription streamBasicDescription = { - .mBitsPerChannel = core->hw.info.bits, + .mBitsPerChannel = audio_format_bits(core->hw.info.af), .mBytesPerFrame = core->hw.info.bytes_per_frame, .mBytesPerPacket = core->hw.info.bytes_per_frame, .mChannelsPerFrame = core->hw.info.nchannels, -- 2.53.0
