[Bug 719950] sndio: Add sndio (OpenBSD) audio sinks/sources

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=719950
  GStreamer | gst-plugins-bad | 1.2.1

Sebastian Dröge (slomo) <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #263639|none                        |needs-work
             status|                            |

--- Comment #3 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2013-12-06 08:43:32 UTC ---
Review of attachment 263639:
 --> (https://bugzilla.gnome.org/review?bug=719950&attachment=263639)

::: ext/sndio/gstsndio.c
@@ +39,3 @@
+    return FALSE;
+  /* prefer sndiosink over pulsesink (GST_RANK_PRIMARY + 10) */
+  if (!gst_element_register (plugin, "sndiosink", GST_RANK_PRIMARY + 20,

Just PRIMARY is enough really :)

@@ +73,3 @@
+gst_sndio_getcaps (struct gstsndio *sio, GstCaps * filter)
+{
+  if (sio->cur_caps == NULL) {

The access to cur_caps needs locking. get_caps() can be called from any thread
at any time, so also while the device is opened or closed

@@ +182,3 @@
+           enc->bits,
+           enc->bps * 8,
+           enc->bps > 1 ? (enc->le ? "LE" : "BE") : "");

Take a look at gst_audio_format_build_integer()

@@ +201,3 @@
+  gst_caps_append_structure (caps, s);
+  sio->cur_caps = caps;
+  GST_DEBUG ("caps are %s", gst_caps_to_string(caps));

GST_DEBUG_OBJECT

@@ +237,3 @@
+
+  if (spec->type != GST_AUDIO_RING_BUFFER_FORMAT_TYPE_RAW) {
+      GST_ELEMENT_ERROR (sio, RESOURCE, OPEN_READ_WRITE,

sio->object, not just sio

@@ +242,3 @@
+  }
+  if (!GST_AUDIO_INFO_IS_INTEGER(&spec->info)) {
+      GST_ELEMENT_ERROR (sio, RESOURCE, OPEN_READ_WRITE,

sio->object, not just sio

@@ +247,3 @@
+  }
+  if (GST_AUDIO_INFO_DEPTH(&spec->info) % 8) {
+      GST_ELEMENT_ERROR (sio, RESOURCE, OPEN_READ_WRITE,

sio->object, not just sio

@@ +274,3 @@
+      break;
+  default:
+      GST_ELEMENT_ERROR (sio, RESOURCE, OPEN_READ_WRITE,

sio->object, not just sio

@@ +295,3 @@
+
+  if (!sio_setpar (sio->hdl, &par)) {
+      GST_ELEMENT_ERROR (sio, RESOURCE, OPEN_WRITE,

sio->object, not just sio

@@ +300,3 @@
+  }
+  if (!sio_getpar (sio->hdl, &retpar)) {
+      GST_ELEMENT_ERROR (sio, RESOURCE, OPEN_WRITE,

sio->object, not just sio

@@ +304,3 @@
+      return FALSE;
+  }
+#if 0

No need to make this in a #if 0 block

@@ +305,3 @@
+  }
+#if 0
+  GST_DEBUG ("format = %s, "

GST_DEBUG_OBJECT

@@ +323,3 @@
+      (par.bps > 1 && par.le != retpar.le) ||
+      (par.bits < par.bps * 8 && par.msb != retpar.msb)) {
+      GST_ELEMENT_ERROR (sio, RESOURCE, OPEN_WRITE,

sio->object, not just sio

@@ +336,3 @@
+
+  if (!sio_start (sio->hdl)) {
+    GST_ELEMENT_ERROR (sio->obj, RESOURCE, OPEN_READ_WRITE,

sio->object, not just sio

::: ext/sndio/sndiosink.c
@@ +147,3 @@
+  GstSndioSink *sink = GST_SNDIOSINK (asink);
+
+  return GST_SNDIO_DELAY(&sink->sndio);

The access to the delay fields in the sndio struct probably needs locking

@@ +188,3 @@
+  gstaudiosink_class = (GstAudioSinkClass *) klass;
+
+  parent_class = g_type_class_peek_parent (klass);

G_DEFINE_TYPE() does that for you already

::: ext/sndio/sndiosrc.c
@@ +147,3 @@
+  GstSndioSrc *src = GST_SNDIOSRC (asrc);
+
+  return GST_SNDIO_DELAY(&src->sndio);

Needs some locking for the delay fields in sndio

@@ +187,3 @@
+  gstaudiosrc_class = (GstAudioSrcClass *) klass;
+
+  parent_class = g_type_class_peek_parent (klass);

G_DEFINE_TYPE() does that for you already

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.