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 674865 - directsoundsink: add support for ac3 over spdif
directsoundsink: add support for ac3 over spdif
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.10.x
Other Windows
: Normal enhancement
: 0.11.x
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-04-26 11:36 UTC by Andoni Morales
Modified: 2012-05-09 07:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add support for ac3 over spdif (6.94 KB, patch)
2012-04-26 11:36 UTC, Andoni Morales
needs-work Details | Review
add support for ac3 over spdif (7.52 KB, patch)
2012-05-08 14:30 UTC, Andoni Morales
committed Details | Review
force 48kHz for AC-3 over spdif (1.02 KB, patch)
2012-05-08 14:32 UTC, Andoni Morales
committed Details | Review
fix assertion (1.55 KB, patch)
2012-05-08 17:02 UTC, Andoni Morales
committed Details | Review

Description Andoni Morales 2012-04-26 11:36:44 UTC
Created attachment 212868 [details] [review]
add support for ac3 over spdif

Initial patch to add support for ac3 over spdif.
Untested since I don't have the proper hardware to test it.
Comment 1 Sebastian Dröge (slomo) 2012-04-26 11:49:11 UTC
Review of attachment 212868 [details] [review]:

Looks good in general :) Does directsound also support DTS/DCA or MP3 or something else? Can be added the same way with the payloaders in the audio library

::: sys/directsound/gstdirectsoundsink.c
@@ +453,3 @@
+      gst_structure_get_boolean (st, "parsed", &parsed);
+      if ((!framed && !parsed) || gst_audio_iec61937_frame_size (&spec) <= 0)
+        goto done;

Might be good to actually check if the device supports AC3 too here

@@ +496,3 @@
+
+  type = GST_BASE_AUDIO_SINK (dsoundsink)->ringbuffer->spec.type;
+  return (type == GST_BUFTYPE_IEC958 || type == GST_BUFTYPE_AC3);

No IEC958 anymore

@@ +835,3 @@
         DXGetErrorString9 (hRes));
     caps =
         gst_caps_subtract (caps, gst_caps_new_simple ("audio/x-iec958", NULL));

No IEC958 anymore

@@ +847,3 @@
   }
 #else
   caps = gst_caps_subtract (caps, gst_caps_new_simple ("audio/x-iec958", NULL));

No IEC958 anymore
Comment 2 Andoni Morales 2012-04-26 15:13:17 UTC
Couldn't we leave support for audio/x-iec958 in the input caps for backward compatibility?

From what I have read here (http://msdn.microsoft.com/en-us/library/windows/hardware/ff538534(v=vs.85).aspx), the supported formats are ac3 and wma-prom, but there is not GstBufferFormatType for it.

Regarding the check, it's already done ::prepare
Comment 3 Sebastian Dröge (slomo) 2012-04-30 07:57:24 UTC
Review of attachment 212868 [details] [review]:

::: sys/directsound/gstdirectsoundsink.c
@@ +429,3 @@
+  GstRingBufferSpec spec = { 0 };
+
+  pad_caps = gst_pad_get_caps_reffed (pad);

Does the getcaps function on the sinkpad only return what the device supports if called in states >= READY?

@@ +432,3 @@
+  if (pad_caps) {
+    ret = gst_caps_can_intersect (pad_caps, caps);
+    gst_caps_unref (pad_caps);

If can_intersect() returns FALSE you should return that immediately without doing the checks below.
Comment 4 Andoni Morales 2012-05-08 14:30:29 UTC
Created attachment 213671 [details] [review]
add support for ac3 over spdif

Update previous patch with the required changes
Comment 5 Andoni Morales 2012-05-08 14:32:03 UTC
Created attachment 213672 [details] [review]
force 48kHz for AC-3 over spdif

AC-3 over spdif requires to be treated as 2-channels, 16-bits, 48 kHz.
Comment 6 Sebastian Dröge (slomo) 2012-05-08 16:04:03 UTC
commit bdaec498cb0203ca801b2f9b8fc2ee3f5ea6ff19
Author: Andoni Morales Alastruey <ylatuya@gmail.com>
Date:   Tue May 8 16:23:42 2012 +0200

    directsoundsink: force 48000 kHz force AC-3 over spdif
    
    AC-3 over spdif must be treated as 2-channel, 16-bit, 48kHz

commit 78ac4dd4e96c8e23aed52176b08f0a91ee8c4778
Author: Andoni Morales Alastruey <ylatuya@gmail.com>
Date:   Thu Apr 26 12:50:09 2012 +0200

    directsoundsink: add support for ac-3 over spdif
Comment 7 Andoni Morales 2012-05-08 17:02:04 UTC
Created attachment 213691 [details] [review]
fix assertion

Fix assertion introduced by previous patch(from gst-plugins-base:3e750315e4)
Comment 8 Sebastian Dröge (slomo) 2012-05-09 07:49:09 UTC
Comment on attachment 213691 [details] [review]
fix assertion

Thanks, now it would be great if you could also port these changes to 0.11 :) If you need help with that, look at the alsasink part or ask me