Add vela alsa lib interface #2970

yangyalei · 2025-01-23T03:22:05Z

Summary

Add vela alsa lib interface

Impact

Introducing ALSA compatible audio subsystem on NuttX.

Testing

Use alsa lib interface play music in allwinnertech&BES platform

Signed-off-by: yangyalei <[email protected]>

nuttxpr · 2025-01-23T03:25:21Z

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides a summary and mentions testing, it lacks crucial details. Here's a breakdown of what's missing:

Summary: While it states "Add vela alsa lib interface," it doesn't explain why this change is necessary. Is it a new feature? Does it fix a bug? What part of the code is affected? How does the "vela alsa lib interface" work and what changes does it introduce? Any related NuttX issues should also be linked.
Impact: Simply stating "nop" is insufficient. Each impact item needs to be addressed explicitly with "NO" or "YES," and if "YES," a description is required. Even if there's no impact, stating "NO" for each item clarifies that these aspects were considered.
Testing: The testing description is vague. It mentions playing music on specific platforms, but doesn't provide details about the build host or the specific boards/configurations used. Critically, it's missing the "before" and "after" testing logs, which are essential for demonstrating the change's effect. What constitutes "works as intended"? What specific tests were run?

To meet the requirements, the PR needs to be revised to include the missing information. For example, the impact section should look like this (even if there's no impact):

* Is new feature added? YES (Adds the vela alsa lib interface)
* Impact on user (will user need to adapt to change)? NO
* Impact on build (will build process change)? NO
* Impact on hardware (will arch(s) / board(s) / driver(s) change)? YES (Affects Allwinnertech and BES platforms - please specify which boards and how)
* Impact on documentation (is update required / provided)? YES (Documentation should be added explaining the vela alsa lib interface and how to use it)
* Impact on security (any sort of implications)? NO
* Impact on compatibility (backward/forward/interoperability)? NO
* Anything else to consider? NO

The testing section needs actual logs and specific build/target details.

yangyalei · 2025-01-23T03:35:42Z

This PR is related to apache/nuttx#15657

cederom · 2025-01-23T03:36:27Z

audioutils/alsa-lib/pcm_dmix.h

+  FAR struct bitsconv_data *bc;
+  FAR struct bitsconv_data *bc_s16;
+  FAR struct chmap_data *cm;
+  FAR SpeexResamplerState *resampler;


This comes from ALSA and needs to stay that way @yangyalei ?

This comes from ALSA and needs to stay that way @yangyalei ?

This is just a simple way to support format convert and resampling... This is not the standard implementation of ALSA; it's just a basic implementation.

Our format check complains here because of mixed case.. but I guess this name is enforced by some standard API and we should not change that to maintain compatibility? :-)

Our format check complains here because of mixed case.. but I guess this name is enforced by some standard API and we should not change that to maintain compatibility? :-)

I used libspeexdsp (BSD license, tested performance is quite good) for resampling, but the interface definitions of libspeexdsp follow camel case naming convention. I have discussed this with @xiaoxiang781216, it's OK.

cederom · 2025-01-23T03:37:11Z

Thank you @yangyalei amazing work!! :-)

raiden00pl · 2025-01-23T09:38:17Z

audioutils/speexdsp/Kconfig

+
+config AUDIOUTILS_SPEEXDSP
+	bool "Audio libspeexdsp Library"
+	default n


depends on ALLOW_BSD_COMPONENTS

raiden00pl

what is the license of this code? is it somehow based on alsa-lib ? Is it GPL?

If this code is GPL then it can't be merged. GPL licensed code cannot be hosted in ASF projects.

xiaoxiang781216 · 2025-01-23T10:05:18Z

what is the license of this code? is it somehow based on alsa-lib ? Is it GPL?

If this code is GPL then it can't be merged. GPL licensed code cannot be hosted in ASF projects.

it's rewrite from scratch, but follow alsa-lib public API, just like nuttx v4l2 driver framework:
https://github.com/apache/nuttx/blob/master/include/sys/videoio.h
The implementation is totally different from https://git.alsa-project.org/?p=alsa-lib.git;a=tree.

jerpelea · 2025-01-23T16:02:01Z

what is the license of this code? is it somehow based on alsa-lib ? Is it GPL?
If this code is GPL then it can't be merged. GPL licensed code cannot be hosted in ASF projects.

it's rewrite from scratch, but follow alsa-lib public API, just like nuttx v4l2 driver framework: https://github.com/apache/nuttx/blob/master/include/sys/videoio.h The implementation is totally different from https://git.alsa-project.org/?p=alsa-lib.git;a=tree.

I will run a scan after merge

raiden00pl · 2025-01-23T16:08:34Z

audioutils/alsa-lib/bits_convert.c

+      const int16_t *src;
+      int32_t *dst;


raiden00pl · 2025-01-23T16:08:50Z

audioutils/alsa-lib/bits_convert.c

+          src = (const int16_t *)in_data;
+          dst = (int32_t *)bc->bitsconv_buf;


raiden00pl · 2025-01-23T16:08:59Z

audioutils/alsa-lib/bits_convert.c

+      int32_t *src;
+      int16_t *dst;


raiden00pl · 2025-01-23T16:09:20Z

audioutils/alsa-lib/bits_convert.c

+          src = (int32_t *)in_data;
+          dst = (int16_t *)bc->bitsconv_buf;


raiden00pl · 2025-01-23T16:09:46Z

audioutils/alsa-lib/channels_map.c

+  in = (const int16_t *)in_data;
+  out = (int16_t *)cm->chmap_buf;


raiden00pl · 2025-01-23T16:09:58Z

audioutils/alsa-lib/channels_map.c

+  in = (const int32_t *)in_data;
+  out = (int32_t *)cm->chmap_buf;


raiden00pl · 2025-01-23T16:11:27Z

audioutils/alsa-lib/pcm.c

+ * Public Functions
+ ****************************************************************************/
+
+const char *snd_pcm_name(snd_pcm_t *pcm)


FAR and in other places

raiden00pl · 2025-01-23T16:18:23Z

audioutils/alsa-lib/pcm.c

+      return NULL;
+    }
+
+  const char *_snd_pcm_format_descriptions[] =


this file looks like highly inspired by https://github.com/alsa-project/alsa-lib/blob/master/src/pcm/pcm.c#L2001

in order to maintain compatibility?

I doubt that compatibility is a sufficient reason not to name this implementation as "derivative GPL work".
Even internal static functions are called the same like in alsa-lib.

raiden00pl · 2025-01-23T16:25:36Z

I'm definitely not licences expert so I don't know if licensing this code to Apache is OK or not.

But for sure caution is required here. GPL is like virus.
Where does "interface inspiration" ends and "derivative work" begin? I see a lot of identical fragments here as in https://github.com/alsa-project/alsa-lib/tree/master/src not only in headers, so I would wounder if we aren't dealing here with "derivative GPL work".

raiden00pl · 2025-01-23T18:31:00Z

it's rewrite from scratch, but follow alsa-lib public API, just like nuttx v4l2 driver framework:

If this is rewriten based on alsa-lib then this is a derivative work. If this is a derivative work of GPL, then this code is GPL. If this code is GPL then it can't be relicensed to Apache.

That's how I understand copyleft licences. I would be happy if there is someone more competent than me and says otherwise.

cederom · 2025-01-24T00:05:18Z

maybe we should ask ALSA project directly to have no doubts?

if the interface is compatible but its a rewrite from scratch then should be no problem? how otherwise create alternatives?

raiden00pl · 2025-01-24T05:21:20Z

The code structure is very similar to alsa-lib, some code fragments are the same, many private functions has the same names. This is not just a compatible interface, but highly inspired on alsa-lib work. Asking the alsa-lib team won't change anything here if it's a derivative work of GPL. I doubt they relicence it to permisive license, I doubt if its even possible.

If you want to avoid such problems you have to write EVERYTHING from scratch, the only thing you can take from GPL project are PUBLIC header interfaces.

yangyalei added 2 commits January 23, 2025 11:19

libspeexdsp: Import speexdsp library

739acea

Signed-off-by: yangyalei <[email protected]>

Alsa-lib: add alsa interface with dmix

c0e1d06

Signed-off-by: yangyalei <[email protected]>

github-actions bot added Area: Audio Size: XL labels Jan 23, 2025

yangyalei mentioned this pull request Jan 23, 2025

Audio: modify audio driver satisfy nuttx alsa apache/nuttx#15657

Open

cederom reviewed Jan 23, 2025

View reviewed changes

raiden00pl reviewed Jan 23, 2025

View reviewed changes

raiden00pl requested changes Jan 23, 2025

View reviewed changes

raiden00pl reviewed Jan 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add vela alsa lib interface #2970

Add vela alsa lib interface #2970

yangyalei commented Jan 23, 2025 •

edited by cederom

Loading

nuttxpr commented Jan 23, 2025

yangyalei commented Jan 23, 2025

cederom Jan 23, 2025

yangyalei Jan 23, 2025

cederom Jan 23, 2025

yangyalei Jan 23, 2025

cederom commented Jan 23, 2025

raiden00pl Jan 23, 2025

raiden00pl left a comment

xiaoxiang781216 commented Jan 23, 2025 •

edited

Loading

jerpelea commented Jan 23, 2025

raiden00pl Jan 23, 2025

raiden00pl Jan 23, 2025

raiden00pl Jan 23, 2025

raiden00pl Jan 23, 2025

raiden00pl Jan 23, 2025

raiden00pl Jan 23, 2025

raiden00pl Jan 23, 2025

raiden00pl Jan 23, 2025

cederom Jan 23, 2025

raiden00pl Jan 23, 2025

raiden00pl commented Jan 23, 2025 •

edited

Loading

raiden00pl commented Jan 23, 2025 •

edited

Loading

cederom commented Jan 24, 2025

raiden00pl commented Jan 24, 2025

		src = (const int16_t *)in_data;
		dst = (int32_t *)bc->bitsconv_buf;

		in = (const int16_t *)in_data;
		out = (int16_t *)cm->chmap_buf;

		in = (const int32_t *)in_data;
		out = (int32_t *)cm->chmap_buf;

Add vela alsa lib interface #2970

Are you sure you want to change the base?

Add vela alsa lib interface #2970

Conversation

yangyalei commented Jan 23, 2025 • edited by cederom Loading

Summary

Impact

Testing

nuttxpr commented Jan 23, 2025

yangyalei commented Jan 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cederom commented Jan 23, 2025

Choose a reason for hiding this comment

raiden00pl left a comment

Choose a reason for hiding this comment

xiaoxiang781216 commented Jan 23, 2025 • edited Loading

jerpelea commented Jan 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raiden00pl commented Jan 23, 2025 • edited Loading

raiden00pl commented Jan 23, 2025 • edited Loading

cederom commented Jan 24, 2025

raiden00pl commented Jan 24, 2025

yangyalei commented Jan 23, 2025 •

edited by cederom

Loading

xiaoxiang781216 commented Jan 23, 2025 •

edited

Loading

raiden00pl commented Jan 23, 2025 •

edited

Loading

raiden00pl commented Jan 23, 2025 •

edited

Loading