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 632118 - [alsasink] probe does not work
[alsasink] probe does not work
Status: RESOLVED NOTABUG
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-10-14 04:33 UTC by Pierre Bossart
Modified: 2010-10-28 18:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch against git master to force device probe (3.40 KB, patch)
2010-10-14 04:33 UTC, Pierre Bossart
needs-work Details | Review

Description Pierre Bossart 2010-10-14 04:33:31 UTC
Created attachment 172326 [details] [review]
patch against git master to force device probe

gstalsa.c contains code to probe actual capabilities of the alsa driver, but this code is never called. In gstalsasink.c, the getcaps routine always returns the default template caps since the handle is not opened.

I hacked a small patch to open the driver and force the probe. For example, the x-iec958 cap is only added if the iec958 device can actually be opened.

The probe is limited since there's no means for now to query what ALSA actually supports. If/when ALSA provides an API to query what codecs are supported by the SPDIF or HDMI device, this probe will need to be extended.

Comments welcome.
-Pierre
Comment 1 Sebastian Dröge (slomo) 2010-10-19 14:12:16 UTC
Comment on attachment 172326 [details] [review]
patch against git master to force device probe

I don't think this patch is correct. The device should only be opened in states >= READY. If getcaps is called in NULL state now the device would be opened.

Why do you want to place iec958 to the beginning of the caps?
Comment 2 Pierre Bossart 2010-10-19 23:26:30 UTC
the existing code isn't correct either, the checks on the actual caps isn't done at all...I don't care when it's done, all I want is the checks to be done. 

the caps placement doesn't seem to matter, I initially thought it would be used by autoconvert as a hint, but really what matters is the rank. You can discard this.
Comment 3 Sebastian Dröge (slomo) 2010-10-20 08:12:52 UTC
Ok, but your patch only moves the opening of the device to the getcaps function and adds a check to not open it twice. What do you mean by check on the actual caps?
Comment 4 Pierre Bossart 2010-10-20 21:39:21 UTC
gst_alsa_probe_supported_formats is never called with the current code since the handle is NULL. The comments say it 'Takes the template caps and returns the subset which is actually supported by this device', but this never happens...

Forcing the opening of the device in getcaps actually enables the filtering on the template caps. There might be more kosher ways to do it but at least this hack enables the functionality, e.g. no x-iec cap if the iec958 device is already busy.
Comment 5 Sebastian Dröge (slomo) 2010-10-25 13:01:55 UTC
There should be a handle after the device was opened in NULL->READY. If you call getcaps then you'll get the complete, supported caps. If you call getcaps before that you'll only get the generic template caps. That's the same behaviour as for example for v4l2src.

What's the bug you're trying to solve here? When does it happen that getcaps is called in NULL state for negotiation?
Comment 6 Pierre Bossart 2010-10-26 22:14:56 UTC
example with an iec958 device that's busy:

with git master
gst-launch filesrc location=~/AURAL/ac3/Bjorn_Lynne-The_Fairy_Woods.ac3 ! ac3iec958 ! alsasink
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
ERROR: from element /GstPipeline:pipeline0/GstAlsaSink:alsasink0: Could not open resource for writing.
Additional debug info:
gstalsasink.c(756): gst_alsasink_prepare (): /GstPipeline:pipeline0/GstAlsaSink:alsasink0:
Could not open IEC958 (SPDIF) device for playback
ERROR: pipeline doesn't want to preroll.
Setting pipeline to NULL ...


with suggested work-around
gst-launch filesrc location=~/AURAL/ac3/Bjorn_Lynne-The_Fairy_Woods.ac3 ! ac3iec958 ! alsasink
0:00:00.028269053  8659  0x8808070 ERROR           GST_PIPELINE ./grammar.y:614:gst_parse_perform_link: could not link ac3iec0 to alsasink0
WARNING: erroneous pipeline: could not link ac3iec0 to alsasink0

In the default case, the connection is allowed without checking if the device is available. When you actually probe if the device is available and change the caps accordingly as done with my patch, the pipeline isn't constructed.

I think the second case is more correct... What I wanted to highlight is that the iec958 cap on the sink pad should only be enabled if the device is actually available. Likewise we should only enable in the sink caps the sampling frequencies and formats that are actually supported by the device. This was the intent of the code written in gstalsa.c, but it is unfortunately not called since the handle is not opened or the state isn't correct.

Got it?
Comment 7 Sebastian Dröge (slomo) 2010-10-28 13:24:58 UTC
Yes but that's really the expected behaviour. You can only know what the device really supports after opening it and you must only open devices in states >= READY and not before. You could change the device in states before READY for example.

I don't think there's anything to fix here. You have to set the alsasink to READY if you want to know if something is really supported and if you want to link stuff before you have to make sure to have elements that support the required conversions.
Comment 8 Pierre Bossart 2010-10-28 14:17:07 UTC
If this is the expected behavior, then why on earth does this probing code in gstalsa.c exist in the first place? It's useless if I follow your line of thought. So either the behavior needs to change, or the behavior is correct and this code needs to be removed since it's essentially dead.
Comment 9 Sebastian Dröge (slomo) 2010-10-28 16:29:39 UTC
It isn't useless. You set the element to READY, it will open the device and get a handle and then you ask for caps and get the caps of the device that was opened.
Comment 10 Pierre Bossart 2010-10-28 18:43:03 UTC
If I use the gst-launch command mentioned above, this code is NOT called. I don't see how/why I would need to do something by hand as a work-around to what I perceive is broken.
But whatever, I reported this to be nice and open, I have zero interest in alsasink per se. If you are not listening that's not really a problem for me. Over and out.