After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 719950 - sndio: Port to GStreamer 1.x
sndio: Port to GStreamer 1.x
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.2.1
Other OpenBSD
: Normal enhancement
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-12-06 08:09 UTC by Brad Smith
Modified: 2016-12-21 11:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
sndio plugin (34.52 KB, patch)
2013-12-06 08:09 UTC, Brad Smith
needs-work Details | Review

Description Brad Smith 2013-12-06 08:09:51 UTC
Created attachment 263639 [details] [review]
sndio plugin

Attached is a diff to add the sndio backend to GStreamer 1.2.x.
Comment 1 Brad Smith 2013-12-06 08:20:06 UTC
Why was this changed to bad when all the other sound backends are in the good package? The description of what "bad" means does not match for this plugin either.
Comment 2 Sebastian Dröge (slomo) 2013-12-06 08:25:12 UTC
New plugins first go into gst-plugins-bad until they fulfill all the requirements for gst-plugins-good or -ugly. See http://cgit.freedesktop.org/gstreamer/gstreamer/tree/docs/random/moving-plugins for the requirements.
Comment 3 Sebastian Dröge (slomo) 2013-12-06 08:43:32 UTC
Review of attachment 263639 [details] [review]:

::: 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
Comment 4 Olivier Crête 2013-12-06 20:52:32 UTC
How is this different from the sndio plugin that is already in gst-plugins-bad/ext/sndio ?
Comment 5 Brad Smith 2013-12-06 22:16:36 UTC
Sebastian@
Ok. I understand why now. There are various other additional requirements for the plugin to move from bad to good.

Thank you for the feedback. I will pass it on and see about having fixes incorporated and then submitting a newer diff for further review.

Olivier@
The sndio plugin for GStreamer 0.10.x still has not been forward ported and incorporated into the 1.2.x code base.
Comment 6 Sebastian Dröge (slomo) 2013-12-07 09:05:00 UTC
Also it would be preferrable to have a patch over the 0.10 version of the plugin in general
Comment 7 Alex-P. Natsios 2016-05-18 12:09:30 UTC
Just a heads up, I am taking a shot at this and will be providing a complete patch in the coming days.
Comment 8 Tim-Philipp Müller 2016-12-21 11:42:21 UTC
This plugin has been removed for the time being.

If anyone is still working on a port, or wants to complete a port, we can resurrect it.

Commit: 9b5de053995488d5ddc78c1bf4df651101271d70
URL:    http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=9b5de053995488d5ddc78c1bf4df651101271d70

Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Dec 21 11:00:47 2016 +0200

Remove various unported plugins

If they were not ported after 4+ years it seems unlikely that anybody is
ever going to need them again. They're still in the GIT history if
needed.

https://bugzilla.gnome.org/show_bug.cgi?id=774530