From: Devarsh Thakkar <devarsh.thakkar@xilinx.com>
Date: Tue, 9 Feb 2021 13:16:34 +0000 (-0800)
Subject: ext: alsa: Set buffer time after period time
X-Git-Tag: 1.19.3~511^2~248
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9759810d8206b5f1aa199f98599caec3630a1813;p=platform%2Fupstream%2Fgstreamer.git

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: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/1040>
---

diff --git a/ext/alsa/gstalsasink.c b/ext/alsa/gstalsasink.c
index f48dd71..9bdf93a 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 431b744..a0dccbb 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);