[Bug 794852] New Audio Element based on Csound

[GStreamer (GNOME Bugzilla)]

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

--- Comment #6 from Natanael Mojica <neithanmo at gmail.com> ---
(In reply to Sebastian Dröge (slomo) from comment #2)
> Review of attachment 370395 [details] [review]:


> @@ +330,3 @@
> +          NULL);
> +    }
> +  } else if (csoundGetSizeOfMYFLT () == FLOAT_SAMPLES) {


yeah, it leave us to remove unnecessary defines, and sizeof(float) is more
intuitive


> @@ +379,3 @@
> +  } else if (direction == GST_PAD_SINK) {
> +    gst_structure_set (structure, "channels", G_TYPE_INT,
> +        csoundfilter->cs_ochannels, NULL);
> 

My first attempt was write a transform_caps and a fixed_caps, but follow you
suggestion, we get a better caps negotiation and performance, joining both
methods and adding a condition to verify if the csound instance has already
created in the transform_caps method, removing the fixate_caps virtual method.


> @@ +432,3 @@
> +        gst_buffer_get_size (inbuf) / (csoundfilter->cs_ichannels *
> +        sizeof (MYFLT));
> +    new_size = num_samples * sizeof (MYFLT) * csoundfilter->cs_ochannels;

I have not known this virtual method. Thanks, better result using this. 

> @@ +465,3 @@
> +  csoundfilter->process (csoundfilter, (MYFLT *) omap.data,
> +      csoundfilter->in_bytes, csoundfilter->out_bytes);
> +  gst_buffer_unmap (outbuf, &omap);
> 
> Will it always fill the complete output buffer or would we have to check
> here how much it wrote?

The filter will copy all data in chunks of a fixed size according to the
csound's input buffer size, then will copy the data from the csound's output
buffer, all the data in the same way

> @@ +480,3 @@
> +static void
> +gst_csoundfilter_trans (GstCsoundfilter * csoundfilter,
> +    MYFLT * odata, guint in_bytes, guint out_bytes)
> 
> This function does not seem to be used anywhere?

This function is used and assigned to a process function pointer
(csoundfilter->process = gst_csoundfilter_trans;)

> @@ +483,3 @@
> +{
> +  gsize offset = 0;
> +  while (gst_adapter_available_fast (csoundfilter->in_adapter) >=

 I used available_fast based on the documentation in the GstAdapter class
"Gets the maximum number of bytes that are immediately available without
requiring any expensive operations (like copying the data into a temporary
buffer).",trying to avoid expensive ooperations. with the adapter I copy data
from/to the csound buffers. This buffers are of a fixed size based on the
settings in the csound file (*.csd), and copy or move data are expensive but,
it is the only way to do it. because the csound API does not to leave us to
pass a pointer where write the audio ouputs like another audio libraries like
openal.

> @@ +499,3 @@
> +    va_list valist)
> +{
> +  char result[256];
> 
> Is is guaranteed that 256 is the maximum? Otherwise use g_strdup_vprintf()

thanks !!, works better and it is safe.

> ::: tests/examples/csound/test-csoundfilter.c
> @@ +82,3 @@
> +  sink = gst_element_factory_make ("autoaudiosink", NULL);
> +  caps = gst_caps_from_string ("audio/x-raw,channels=1,rate=44100");
> +  caps2 = gst_caps_from_string ("audio/x-raw,channels=2,rate=44100");
> 
> The second caps filter here seems unneeded. audioconvert should figure out
> something that works together with autoaudiosink

 with the transform_caps changed, the second caps and first caps are
unnecessary, the pipeline figure out what are the best caps for both.

> see that sample rate and in/out channels at least is defined by the csd file?

the user will define the audio parameters inside of the csound file *.csd, So,
they should be considerer during the caps negotiation. Bacause this file is
load and compiled when the csound instance is created(in the start() method ) I
separated the negotiation in two methods, transform_caps and fixated_caps.. But
now, I followed your suggestion, removing the fixated_caps method, and make the
negotiation in the fransform_caps, checking if the csound instance has alredy
created for inmedialy considerer the user settings before the negotiation
procedure finishes.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.