From 9759810d8206b5f1aa199f98599caec3630a1813 Mon Sep 17 00:00:00 2001 From: Devarsh Thakkar Date: Tue, 9 Feb 2021 05:16:34 -0800 Subject: [PATCH] ext: alsa: Set buffer time after period time This because underlying driver may have constraint on buffer size to be dependent on period size, so period time needs to be set first. For e.g. Xilinx ASoC driver requires buffer size to be multiple of period size for it's DMA operation. alsa-utils also set period time first as seen in below commit : https://github.com/alsa-project/alsa-utils/commit/9b621eeac4d55c4e881f093be5b163ca07d01b63 Tested it on zcu106 board with HDMI based record and playback. Also tested on Intel PC using Logitech C920 Webcam mic and ALC887-VD Analog for playback. Part-of: --- ext/alsa/gstalsasink.c | 94 +++++++++++++++++++++++++++--------------- ext/alsa/gstalsasrc.c | 25 ++++++++++- 2 files changed, 85 insertions(+), 34 deletions(-) diff --git a/ext/alsa/gstalsasink.c b/ext/alsa/gstalsasink.c index f48dd71ce4..9bdf93a72a 100644 --- a/ext/alsa/gstalsasink.c +++ b/ext/alsa/gstalsasink.c @@ -492,40 +492,68 @@ retry: GST_DEBUG_OBJECT (alsa, "periods min %u, max %u", min, max); } #endif - - /* now try to configure the buffer time and period time, if one - * of those fail, we fall back to the defaults and emit a warning. */ - if (buffer_time != -1 && !alsa->iec958) { - /* set the buffer time */ - if ((err = snd_pcm_hw_params_set_buffer_time_near (alsa->handle, params, - &buffer_time, NULL)) < 0) { - GST_ELEMENT_WARNING (alsa, RESOURCE, SETTINGS, (NULL), - ("Unable to set buffer time %i for playback: %s", - buffer_time, snd_strerror (err))); - /* disable buffer_time the next round */ - buffer_time = -1; - goto retry; + if (!alsa->iec958) { + /* Following pulseaudio's approach in + * https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/commit/557c4295107dc7374c850b0bd5331dd35e8fdd0f + * we'll try various configuration to set the buffer time and period time as some + * driver can be picky on the order of the calls. + */ + if (buffer_time != -1 && period_time != -1) { + if (((err = snd_pcm_hw_params_set_buffer_time_near (alsa->handle, params, + &buffer_time, NULL)) >= 0) + && ((err = + snd_pcm_hw_params_set_period_time_near (alsa->handle, params, + &period_time, NULL)) >= 0)) { + GST_DEBUG_OBJECT (alsa, "buffer time %u period time %u set correctly", + buffer_time, period_time); + alsa->buffer_time = buffer_time; + alsa->period_time = period_time; + goto buffer_period_set; + } + if (((err = snd_pcm_hw_params_set_period_time_near (alsa->handle, params, + &period_time, NULL)) >= 0) + && ((err = + snd_pcm_hw_params_set_buffer_time_near (alsa->handle, params, + &buffer_time, NULL)) >= 0)) { + GST_DEBUG_OBJECT (alsa, "period time %u buffer time %u set correctly", + period_time, buffer_time); + alsa->buffer_time = buffer_time; + alsa->period_time = period_time; + goto buffer_period_set; + } } - GST_DEBUG_OBJECT (alsa, "buffer time %u", buffer_time); - alsa->buffer_time = buffer_time; - } - if (period_time != -1 && !alsa->iec958) { - /* set the period time */ - if ((err = snd_pcm_hw_params_set_period_time_near (alsa->handle, params, - &period_time, NULL)) < 0) { - GST_ELEMENT_WARNING (alsa, RESOURCE, SETTINGS, (NULL), - ("Unable to set period time %i for playback: %s", - period_time, snd_strerror (err))); - /* disable period_time the next round */ - period_time = -1; - goto retry; + /* now try to configure the buffer time and period time independently, if one + * of those fail, we fall back to the defaults and emit a warning. */ + if (buffer_time != -1) { + /* set the buffer time */ + if ((err = snd_pcm_hw_params_set_buffer_time_near (alsa->handle, params, + &buffer_time, NULL)) < 0) { + GST_ELEMENT_WARNING (alsa, RESOURCE, SETTINGS, (NULL), + ("Unable to set buffer time %i for playback: %s", + buffer_time, snd_strerror (err))); + /* disable buffer_time the next round */ + buffer_time = -1; + goto retry; + } + GST_DEBUG_OBJECT (alsa, "buffer time %u set correctly", buffer_time); + alsa->buffer_time = buffer_time; } - GST_DEBUG_OBJECT (alsa, "period time %u", period_time); - alsa->period_time = period_time; - } - - /* Set buffer size and period size manually for SPDIF */ - if (G_UNLIKELY (alsa->iec958)) { + if (period_time != -1) { + /* set the period time */ + if ((err = snd_pcm_hw_params_set_period_time_near (alsa->handle, params, + &period_time, NULL)) < 0) { + GST_ELEMENT_WARNING (alsa, RESOURCE, SETTINGS, (NULL), + ("Unable to set period time %i for playback: %s", + period_time, snd_strerror (err))); + /* disable period_time the next round */ + period_time = -1; + goto retry; + } + GST_DEBUG_OBJECT (alsa, "period time %u set correctly", period_time); + alsa->period_time = period_time; + } + } else { + /* Set buffer size and period size manually for SPDIF */ snd_pcm_uframes_t buffer_size = SPDIF_BUFFER_SIZE; snd_pcm_uframes_t period_size = SPDIF_PERIOD_SIZE; @@ -534,7 +562,7 @@ retry: CHECK (snd_pcm_hw_params_set_period_size_near (alsa->handle, params, &period_size, NULL), period_size); } - +buffer_period_set: /* write the parameters to device */ CHECK (snd_pcm_hw_params (alsa->handle, params), set_hw_params); diff --git a/ext/alsa/gstalsasrc.c b/ext/alsa/gstalsasrc.c index 431b74451e..a0dccbb43c 100644 --- a/ext/alsa/gstalsasrc.c +++ b/ext/alsa/gstalsasrc.c @@ -415,7 +415,29 @@ set_hwparams (GstAlsaSrc * alsa) GST_DEBUG_OBJECT (alsa, "periods min %u, max %u", min, max); } #endif - + /* Following pulseaudio's approach in + * https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/commit/557c4295107dc7374c850b0bd5331dd35e8fdd0f + * we'll try various configuration to set the buffer time and period time as some + * driver can be picky on the order of the calls. + */ + if (alsa->period_time != -1 && alsa->buffer_time != -1) { + if ((snd_pcm_hw_params_set_buffer_time_near (alsa->handle, params, + &alsa->buffer_time, NULL) >= 0) + && (snd_pcm_hw_params_set_period_time_near (alsa->handle, params, + &alsa->period_time, NULL) >= 0)) { + GST_DEBUG_OBJECT (alsa, "buffer time %u period time %u set correctly", + alsa->buffer_time, alsa->period_time); + goto buffer_period_set; + } + if ((snd_pcm_hw_params_set_period_time_near (alsa->handle, params, + &alsa->period_time, NULL) >= 0) + && (snd_pcm_hw_params_set_buffer_time_near (alsa->handle, params, + &alsa->buffer_time, NULL) >= 0)) { + GST_DEBUG_OBJECT (alsa, "period time %u buffer time %u set correctly", + alsa->period_time, alsa->buffer_time); + goto buffer_period_set; + } + } if (alsa->buffer_time != -1) { /* set the buffer time */ CHECK (snd_pcm_hw_params_set_buffer_time_near (alsa->handle, params, @@ -429,6 +451,7 @@ set_hwparams (GstAlsaSrc * alsa) GST_DEBUG_OBJECT (alsa, "period time %u", alsa->period_time); } +buffer_period_set: /* write the parameters to device */ CHECK (snd_pcm_hw_params (alsa->handle, params), set_hw_params);