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 779805 - alsasink: add SND_PCM_FORMAT_DSD support
alsasink: add SND_PCM_FORMAT_DSD support
Status: RESOLVED WONTFIX
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-03-09 13:56 UTC by nekomatu
Modified: 2018-05-06 10:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
adds SND_PCM_FORMAT_DSD series (4.62 KB, patch)
2017-03-09 13:56 UTC, nekomatu
none Details | Review
adds SND_PCM_FORMAT_DSD series (3.21 KB, patch)
2017-03-09 13:58 UTC, nekomatu
none Details | Review
[RFC] v2-0001-audio-format-Add-GST_AUDIO_FORMAT_DSD-series (4.86 KB, patch)
2017-03-10 08:54 UTC, nekomatu
none Details | Review
v2-0002-alsa-Add-GST_AUDIO_FORMAT_DSD-series (2.07 KB, patch)
2017-03-10 08:56 UTC, nekomatu
none Details | Review

Description nekomatu 2017-03-09 13:56:55 UTC
Created attachment 347558 [details] [review]
adds SND_PCM_FORMAT_DSD series

What's the patch?
===============
The patch just adds SND_PCM_FORMAT_DSD series.
SND_PCM_FORMAT_DSD was defined by alsa-lib v1.0.29.

https://www.alsa-project.org/main/index.php/Changes_v1.0.28_v1.0.29
pcm: add new 32-bit DSD sample format

Incidentally,  SND_PCM_FORMAT_LAST = SND_PCM_FORMAT_DSD_U32_BE.
It's mean follow pcm_format of alsa-lib.
http://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.html


Howto native-dsd playback?
====================
Sorry, we can't play dsd on gstreamer now.
because, we don't have dsd decoder elements.

I confirmed combination software stack.
MPD 0.20.2+, alsa-lib v1.0.29+, kernel 3.18+ and supported sound device.
http://git.musicpd.org/cgit/master/mpd.git/plain/NEWS?h=v0.20.2

If you want to more information, Please refer to the URL
https://github.com/lintweaker/xmos-native-dsd


Technical description
=========================
* silence value is 0x69
Refer to Line106
https://www.ffmpeg.org/doxygen/2.4/dsddec_8c_source.html
* alsa-lib define
http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/pcm/pcm.c;h=a16fd86fb3b004ca93bb07b568c4724182d5dd61;hb=ad188bbf7813eab6f42dcdf617aa947107118857#l1800

http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/pcm/pcm.c;h=a16fd86fb3b004ca93bb07b568c4724182d5dd61;hb=ad188bbf7813eab6f42dcdf617aa947107118857#l1862

* other information
UsbAudioClass3.0 define DSD format
2.3.1.6.5 DSD FORMAT
Frmts30.pdf at USB Audio Devices Rev. 3.0 Spec and Adopters Agreement 
http://www.usb.org/developers/docs/devclass_docs/


My first send patch to Gstreamer community.
Let me know anything problems.
Thank you.
Comment 1 nekomatu 2017-03-09 13:58:06 UTC
Created attachment 347559 [details] [review]
adds SND_PCM_FORMAT_DSD series
Comment 2 nekomatu 2017-03-09 13:59:56 UTC
I'm sorry.Wrong select component
correct component is gst-plugin-base.
Comment 3 Sebastian Dröge (slomo) 2017-03-09 15:12:25 UTC
Review of attachment 347558 [details] [review]:

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
Comment 4 nekomatu 2017-03-10 08:54:40 UTC
Created attachment 347600 [details] [review]
[RFC] v2-0001-audio-format-Add-GST_AUDIO_FORMAT_DSD-series

* Modify broke ABI
* FIXME about pack define
Comment 5 nekomatu 2017-03-10 08:56:52 UTC
Created attachment 347601 [details] [review]
v2-0002-alsa-Add-GST_AUDIO_FORMAT_DSD-series
Comment 6 nekomatu 2017-03-10 08:59:01 UTC
Thank you for your review.

> Is this really DSD silence? Or just some number?
Yes.SILENT_DSD is same mean SILENT_0. so, Mute of PCM is 0x00. Mute of DSD is 0x69.
It is followed kernel.
http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/sound/core/pcm_misc.c#n143
http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/sound/usb/endpoint.c#n614

> The pack and unpack functions are all wrong for DSD.
I'm sorry.acutually, I don't understand pack and unpack.

> 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.
Maybe correct.
DSD formats don't have compatibilty PCM formats.
It's completely different PCM formats.
and
I understand and confirm only DSD_U32_BE.
so, I don't know that DSD can convert to other formats(e.g, DSD_U8 to DSD_U32_BE, DSD_U16_LE to DSD_U32_BE).

but.. I'ms sorry. I can't understand howto change code :-(
Could you please tell me that how do I change it?
I wrote FIXME at patch v2.

> This is breaking ABI
Oops! It's very important.

Have a nice weekend.
Comment 7 Sebastian Dröge (slomo) 2017-03-10 12:28:37 UTC
The point of the pack/unpack functions in libgstaudio is to have a common format that every other format can be converted to. These are currently S32 (for all integer formats) and F64 (for all floating point formats) in native endianness. It's a requirement that these functions exist and do the right thing for all audio/x-raw formats defined by libgstaudio, so that elements can have a generic way of handling all kinds of raw audio formats.

So you have 3 options here now
a) define a third common format here for DSD, so that we would have S32, F64 and DSD_U32. And then also add conversion between these 3 formats.
b) have the functions for DSD convert from/to S32 or F64
c) don't use audio/x-raw and don't add these as formats to libgstaudio, but handle them like any compressed format (call it e.g. audio/x-dsd)
Comment 8 nekomatu 2017-03-13 03:14:52 UTC
Thank you for quick response.

I've understood that I have to confirm something.

No1) Can those convert it.
DSD_U8,DSD_U{16,32}_{LE,BE}
If I resolve it, May I ask alsa-dev.

No2) x-raw is PCM? is it correct?
https://gstreamer.freedesktop.org/documentation/plugin-development/advanced/media-types.html

No3) convert is not decode. is it correct?
I think that pcm and dsd are not compatible. 
so, I think those can't convert.It's mean not convert,but decode.
because, I think that I can't select optionB

If 2 and 3 is true:
We need to define audio/x-dsd.
However, I may need to clear No1.

This work is difficult more than I have a imagine.
so, This work may be a painting.
Comment 9 Sebastian Dröge (slomo) 2017-03-13 08:47:02 UTC
(In reply to n-matsumoto from comment #8)

> No1) Can those convert it.
> DSD_U8,DSD_U{16,32}_{LE,BE}
> If I resolve it, May I ask alsa-dev.

I don't understand your question. You mean if you can convert between these formats? Probably. But even then you still need to implement conversion from/to PCM (F64 and S32).

> No2) x-raw is PCM? is it correct?
> https://gstreamer.freedesktop.org/documentation/plugin-development/advanced/
> media-types.html

It's currently only PCM, yes. But as DSD is also raw audio, it would also fit in there IMHO.

> No3) convert is not decode. is it correct?
> I think that pcm and dsd are not compatible. 
> so, I think those can't convert.It's mean not convert,but decode.
> because, I think that I can't select optionB

PCM and DSD are both (raw) audio formats so (lossy!) conversion between them is possible. I wouldn't call any of it decoding/encoding though as they are both equivalent formats, none of the two is the "encoded" one.
Comment 10 nekomatu 2017-03-13 09:37:07 UTC
(In reply to Sebastian Dröge (slomo) from comment #9)
> > No1) Can those convert it.
> > DSD_U8,DSD_U{16,32}_{LE,BE}
> > If I resolve it, May I ask alsa-dev.
> 
> I don't understand your question. You mean if you can convert between these
> formats? Probably. But even then you still need to implement conversion
> from/to PCM (F64 and S32).
I'm sorry. That's right!
I look at MPD implementation...
http://git.musicpd.org/cgit/master/mpd.git/tree/src/pcm/Dsd16.hxx?id=v0.20.6#n31
http://git.musicpd.org/cgit/master/mpd.git/tree/src/pcm/Dsd32.hxx?id=v0.20.6#n31

http://git.musicpd.org/cgit/master/mpd.git/tree/src/pcm/Dsd32.cxx?id=v0.20.6

In Gstreamer case:
Maybe we need DSD_U32. 
(I can't create source code.because I don't understand pack/unpack and native endianess equivalents enough yet)


> > No2) x-raw is PCM? is it correct?
> > https://gstreamer.freedesktop.org/documentation/plugin-development/advanced/
> > media-types.html
> 
> It's currently only PCM, yes. But as DSD is also raw audio, it would also
> fit in there IMHO.
> 
> > No3) convert is not decode. is it correct?
> > I think that pcm and dsd are not compatible. 
> > so, I think those can't convert.It's mean not convert,but decode.
> > because, I think that I can't select optionB
> 
> PCM and DSD are both (raw) audio formats so (lossy!) conversion between them
> is possible. I wouldn't call any of it decoding/encoding though as they are
> both equivalent formats, none of the two is the "encoded" one.
This is my opinion that your opposite opinion.
because, ffmpeg and mpd have dsd to pcm decoder.
http://git.musicpd.org/cgit/master/mpd.git/tree/src/pcm/dsd2pcm?id=v0.20.6
https://lists.ffmpeg.org/pipermail/ffmpeg-cvslog/2014-April/076427.html
Comment 11 nekomatu 2017-09-14 03:48:29 UTC
I'm sorry.I don't work it.

I'm going to leave my company.so,I can't access this mail address and purge it.

Bugzilla can not delete a account.but can change mail address.
I'm going to change form company mail to private mail.
This ticket was created my job time.
Therefore, I'm hard that continue to develop in my private time.

Please let me know if you have any questions.
Thank you.
Comment 12 Mathieu Duponchelle 2018-05-06 10:55:32 UTC
Closing this, leaving this open makes no sense if no one is willing to work on it, thanks for your efforts anyway!

Note that DSD / DSF files can now be played back using gst-libav.