GNOME Bugzilla – Bug 674921
[0.10] alsasink: use the iec958 payloader to support non-payloaded input streams
Last modified: 2013-04-20 21:47:55 UTC
Created attachment 212933 [details] [review] alsa: support non iec958 payloaded streams Make use internally of the iec958 payloader to support non-payloaded input streams. The patch is not tested properly as I don't have the proper hardware for it.
Review of attachment 212933 [details] [review]: ::: ext/alsa/gstalsasink.c @@ +356,3 @@ + ret = gst_caps_can_intersect (pad_caps, caps); + gst_caps_unref (pad_caps); + } You should check if if can_intersect() returned FALSE, and if it did don't do any of the checks below.
Created attachment 213305 [details] [review] alsasink: use the iec958 payloader tu support non-payloaded input streams
commit c6409806c144ee5070f8d623093dcb3b894ed3d5 Author: Andoni Morales Alastruey <ylatuya@gmail.com> Date: Fri Apr 27 10:19:15 2012 +0200 alsasink: use the iec958 payloader to support non-payloaded input streams
This breaks playback of gst-launch-0.10 playbin2 uri=http://www.sofrito.co.uk/Music/Soweto_Disco_Promo/Oh_yeh_Soweto_excerpt.mp3 for me on my laptop. I just get noise.
Created attachment 214134 [details] dbg.log.xz
Created attachment 214137 [details] [review] alsasink: only check for spdif support in the selected device The first issue is that in prepare, it was checking for "spec->format == GST_IEC958" instead of "alsa->iec958" and that's why the payloaded stream was pushed to the PCM device instead of the iec958 one. But with this fix, if the device supports digital output the sink will always negotiate inside playbin an encoded format (ac-3, dts or mp3 instead of the raw decoded one) and therefore always using the iec958 device which I don't think that's what we want. So I have also changed the probe function to use the device passed in the 'device' property, this way only when device=iec958 is set the pass-through caps will be added to the supported caps. I'm not sure if explained my self correctly and even if it's the correct solution :)
Oh, and like I said before I don't have the correct hardware to test it :(
Thanks for the patch. It fixes the issue with the noise and PCM output, but I'm not sure if it's fully right yet. Can't test the SPDIF bits properly before the weekend though.
Isn't it possible to just use the "default" device if none is selected and somehow detect if that can do iec958 or not?
Another thing: alsasink now doesn't handle "audio/x-iec958" any longer, which means things will break for people who were using that (e.g. with ac3iec958). Was that removed on purpose?
Comment on attachment 214137 [details] [review] alsasink: only check for spdif support in the selected device commit 2434f2932bd02b627f4498534531c218ead1d102 Author: Andoni Morales Alastruey <ylatuya@gmail.com> Date: Tue May 15 19:21:15 2012 +0200 alsasink: check for spdif support only in the current device
(In reply to comment #10) > Another thing: alsasink now doesn't handle "audio/x-iec958" any longer, which > means things will break for people who were using that (e.g. with ac3iec958). > Was that removed on purpose? To me that's something that should be kept for backwards compatibility but Sebastian suggested removing it here https://bugzilla.gnome.org/show_bug.cgi?id=674865
> > Another thing: alsasink now doesn't handle "audio/x-iec958" any longer, which > > means things will break for people who were using that (e.g. with ac3iec958). > > Was that removed on purpose? > > To me that's something that should be kept for backwards compatibility but > Sebastian suggested removing it here > https://bugzilla.gnome.org/show_bug.cgi?id=674865 Can't really do that unless the functionality was completely broken before (but I don't think that's the case here.)
If I wasn't too lazy I would be reverting these in 0.10 now. But I am, and there's not probably not going to be another 0.10 release, so let's just close this.