[Bug 779805] alsasink: add SND_PCM_FORMAT_DSD support

[GStreamer (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #347558|none                        |needs-work
             status|                            |

--- Comment #3 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 347558:
 --> (https://bugzilla.gnome.org/review?bug=779805&attachment=347558)

Thanks for your patch!

::: gst-libs/gst/audio/audio-format.c
@@ +225,3 @@
 #define SILENT_0         { 0, 0, 0, 0, 0, 0, 0, 0 }
 #define SILENT_U8        { 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80 }
+#define SILENT_DSD       { 0x69, 0x69, 0x69, 0x69, 0x69, 0x69, 0x69, 0x69 }

Is this really DSD silence? Or just some number?

@@ +248,3 @@
            PACK_U8),
+       MAKE_FORMAT (DSD_U8, "8-bit unsigned DSD audio", UINT, 0, 8, 8,
SILENT_DSD,
+           PACK_U8),

The pack and unpack functions are all wrong for DSD. You would have to
implement DSD<->PCM conversion in there. Or define one of the DSD formats (the
U32 with native endianness I guess) as the unpack format... and then implement
conversion from the other DSD formats to that one (in the pack/unpack
functions), and additionally implement conversion from that to PCM so
everything works as expected.

Without this, it won't be a proper raw audio format as you can't use the
conversion features from libgstaudio on it. And things will break because of
that.


The alternative would be to consider DSD a different format and not raw audio.
Then it would also not have to be added to libgstaudio and we could handle it
exactly the same as any compressed format.

::: gst-libs/gst/audio/audio-format.h
@@ +113,3 @@
   GST_AUDIO_FORMAT_U32BE,
+  GST_AUDIO_FORMAT_DSD_U32LE,
+  GST_AUDIO_FORMAT_DSD_U32BE,

This is breaking ABI: you would have to add the new enum values at the end of
the enum, so that all the existing ones don't change their numerical value

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