[Bug 759389] fakesrc ! wasapisink and any other combination raises The stream is in the wrong format

[GStreamer (GNOME Bugzilla)]

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

--- Comment #12 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
(In reply to Thomas Roos from comment #11)
> Created attachment 325478 [details] [review]
> fix basic stuff - 2nd try
> 
> +  gst_element_class_add_pad_template (gstelement_class,
> +      gst_static_pad_template_get (&sink_template));
> 
> Why this change? It was intentionally changed just a while ago
> -> otherwise I will get an warning
> "C:/mingw-w64/i686-4.8.1-posix-sjlj-rt_v3-rev2/mingw32/i686-w64-mingw32/
> include/gstreamer-1.0/gst/gstelement.h:660:25: note: expected 'struct
> GstPadTemplate *' but argument is of type 'struct GstStaticPadTemplat
> e *'
>  void                    gst_element_class_add_pad_template     
> (GstElementClass *klass, GstPadTemplate *templ);"

It was gst_element_class_add_*static*_pad_template(), which was added with
1.8.0. That gives you a warning?


> @@ +111,3 @@
> +  if (FAILED(hr)) {
> +        GST_ELEMENT_ERROR(self, RESOURCE, OPEN_READ,
> +                ("CoInitializeEx: %ld", hr), (NULL));
> 
> GST_ELEMENT_ERROR() does not work during init()
> 
> -> dont know the correct gst way of printing err msg then

You could remember in an instance variable that things failed, and then during
the state change from NULL->READY fail like that. Also it makes probably more
sense to do the COM initialization in class_init() (i.e. once per class, not
once per instance).

> @@ +144,3 @@
> +
> +  GST_DEBUG ("gst_wasapi_sink_get_caps filter: %s", gst_caps_to_string
> (filter));
> +  GST_DEBUG ("gst_wasapi_sink_get_caps: %s", gst_caps_to_string (caps));
> 
> You need to filter it against the filter caps if filter!=NULL
> -> added comment TODO

It's not a TODO, it's a bug that will cause assertions. Alternatively you can
just continue returning NULL here, then the base class should do the right
thing.

> @@ +341,2 @@
> +  //TODO: unsure if the calculation is correct
> +  hr = IAudioClient_GetCurrentPadding ((IAudioClient*)self->client,
> &nbOfSamplesInQueue);
> 
> Padding sounds wrong. We're interested in how many samples are currently
> queued
> in WASAPI but not played out yet
> -> I know it sounds wrong, but for me description sounds correct
> https://msdn.microsoft.com/de-de/library/windows/desktop/dd370868%28v=vs.
> 85%29.aspx

Ok

> ::: sys/wasapi/gstwasapisrc.c
> @@ +205,3 @@
> 
> +  hr = IAudioClient_Initialize (self->client, AUDCLNT_SHAREMODE_SHARED,
> AUDCLNT_STREAMFLAGS_EVENTCALLBACK,
> +         spec->buffer_time / 100, 0, (WAVEFORMATEX *) &format, NULL);
> 
> In the sink you do * 10, here you do / 100. What is correct now?
> 
> -> you're right - microsoft use hns (100ns) as unit

Ok :) Should you also check here afterwards if you got the requested buffer
time, and update with the actual value returned?

> @@ +347,3 @@
> +      GST_DEBUG ("gst_wasapi_src_delay");
> +
> +      hr = IAudioCaptureClient_GetNextPacketSize (self->capture_client,
> &nsamples);
> 
> Also doesn't seem to be the correct API. Maybe here you could return the
> difference between how much data you read and the audio clock position? See
> old
> get_time() function
> 
> -> I know it sounds wrong, but for me description sounds correct

ok :)

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