GNOME Bugzilla – Bug 719950
sndio: Port to GStreamer 1.x
Last modified: 2016-12-21 11:42:21 UTC
Created attachment 263639 [details] [review] sndio plugin Attached is a diff to add the sndio backend to GStreamer 1.2.x.
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.
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.
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
How is this different from the sndio plugin that is already in gst-plugins-bad/ext/sndio ?
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.
Also it would be preferrable to have a patch over the 0.10 version of the plugin in general
Just a heads up, I am taking a shot at this and will be providing a complete patch in the coming days.
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