lc3: Moved LC3 #826

koffes · 2022-09-07T13:08:24Z

Moved LC3 from separate repo to sdk-nrfxlib

Signed-off-by: Kristoffer Rist Skøien kristoffer.skoien@nordicsemi.no

CLAassistant · 2022-09-07T13:08:30Z

All committers have signed the CLA.

erikrobstad · 2022-09-08T07:32:02Z

lc3/Kconfig

@@ -0,0 +1,115 @@
+#
+# Copyright (c) 2022 Nordic Semiconductor ASA


Do we need this long license here? Or can we use the short 5-Clause one?

erikrobstad · 2022-09-08T07:37:33Z

lc3/include/sw_codec_lc3.h

+ *
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: Nordic-5-Clause


Can we use the short 5-Clause license?
It looks like lc3/Cmakelists.txt uses the short one

erikrobstad · 2022-09-08T07:39:32Z

lc3/src/sw_codec_lc3.c

+ *
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: Nordic-5-Clause


Can we use short 5-Clause here as well?

greg-fer

The readme.mds and other txt and html files cannot go into nrfxlib in the current form.
As this is a major addition to the nrfxlib and the docs, I suggest the PR to be moved to after 2.1.0.

rlubos · 2022-09-08T09:00:36Z

@greg-fer @carlescufi lc3/doc/T2_LC3_API_Documentation_1.5.pdf - I don't think this file should end up in sdk-nrfxlib either? Something's causing compliance check crash, I strongly suspect this file.

b-gent · 2022-09-08T09:23:47Z

The readme.mds and other txt and html files cannot go into nrfxlib in the current form. As this is a major addition to the nrfxlib and the docs, I suggest the PR to be moved to after 2.1.0.

+1 to remove the milestone from this, documentation needs a thorough review and we are 1 day away from the doc improvement freeze (not mentioning this is not a doc improvement but a whole new lib getting added).

carlescufi · 2022-09-08T09:30:40Z

@greg-fer @b-gent @rlubos will ping you all offline.

tejlmand

lgtm

rlubos · 2022-09-08T09:42:17Z

@koffes Can you sign the CLA (#826 (comment))? Or did you do that already and it's just the CLA check failure (it happens sometimes)?

Moved LC3 from separate repo to sdk-nrfxlib Signed-off-by: Kristoffer Rist Skøien <kristoffer.skoien@nordicsemi.no>

carlescufi · 2022-09-08T10:00:11Z

@koffes Can you sign the CLA (#826 (comment))? Or did you do that already and it's just the CLA check failure (it happens sometimes)?

@rlubos I think it's a one-off because the CLA appears signed in the sdk-nrf PR:
nrfconnect/sdk-nrf#8673 (review)

koffes · 2022-09-08T10:21:00Z

As per the CLA: This work is Submitted on behalf of a third-party T2 Software/Nordic Semiconductor

lc3/README.rst

umapraseeda · 2022-09-08T12:44:38Z

lc3/src/sw_codec_lc3.c

+
+	/* Set unique session to 0 for using the default sharing memory setting.
+	 *
+	 * This could lead to higher heap consumtion, but is able to manipulate


Suggested change

* This could lead to higher heap consumtion, but is able to manipulate

* This could lead to higher heap consumption, but is able to manipulate

lc3/README.rst

umapraseeda · 2022-09-08T12:58:51Z

lc3/codec/inc/LC3API.h

+ *
+ * When LC3_SAMPLE_RATE_ALL and/or LC3FrameSizeBothConfig are used and the
+ * uniqueSessions value is less than the number of (sample rates * frame sizes),
+ * the codec allocates the worse case shared memory required based on the


Suggested change

* the codec allocates the worse case shared memory required based on the

* the codec allocates the worst case shared memory required based on the

Cannot edit this file due to legal reasons.

umapraseeda · 2022-09-08T12:58:57Z

lc3/codec/inc/LC3API.h

+
+/**@brief Closes an LC3 Decoder session.
+ *
+ * This function closes the decodeHandle's session and and releases allocated session memory.


Suggested change

* This function closes the decodeHandle's session and and releases allocated session memory.

* This function closes the decodeHandle's session and releases allocated session memory.

Cannot edit this file due to legal reasons.

koffes

LGTM. I can't approve my own PR.

koffes · 2022-09-08T13:01:28Z

lc3/README.rst

+   :depth: 2
+
+Low Complexity Communication Codec (LC3) is the default software codec for the :ref:`nrf53_audio_app` application, conformant to the `Bluetooth® LE Audio specifications`_ (Bluetooth 5.2, QDID #156294).
+It is used only by this application for encoding and decoding purposes, but can be used with any other audio application developed with the nRF5340 Audio DK.


Not quite precise. I think it is better to refer to license.txt not to duplicate info

Removed line.

Created a readme file with changelog included. Moved the API PDF to FTP and included link in the readme. Moved legal files to top folder. Deleted original doc files. Signed-off-by: Grzegorz Ferenc <Grzegorz.Ferenc@nordicsemi.no>

koffes self-assigned this Sep 7, 2022

koffes requested a review from tejlmand as a code owner September 7, 2022 13:08

koffes assigned tejlmand, carlescufi, rick1082, asbjornsabo and alexsven Sep 7, 2022

koffes unassigned koffes, tejlmand, carlescufi, rick1082, asbjornsabo and alexsven Sep 7, 2022

koffes requested review from alexsven, asbjornsabo, carlescufi, rick1082 and erikrobstad September 7, 2022 13:09

alexsven approved these changes Sep 7, 2022

View reviewed changes

rick1082 approved these changes Sep 7, 2022

View reviewed changes

erikrobstad approved these changes Sep 8, 2022

View reviewed changes

NordicBuilder mentioned this pull request Sep 8, 2022

applications: nrf5340_audio: updated manifest and removed group filter nrfconnect/sdk-nrf#8673

Merged

koffes force-pushed the move_lc3 branch from 2a77f5b to 76e2521 Compare September 8, 2022 08:53

greg-fer requested changes Sep 8, 2022

View reviewed changes

rlubos added this to the 2.1.0 milestone Sep 8, 2022

koffes force-pushed the move_lc3 branch 2 times, most recently from 646e2cc to 4b56752 Compare September 8, 2022 09:20

koffes force-pushed the move_lc3 branch 2 times, most recently from 0427c8e to 1ffecb7 Compare September 8, 2022 09:36

tejlmand approved these changes Sep 8, 2022

View reviewed changes

lc3: Moved LC3

b850bc2

Moved LC3 from separate repo to sdk-nrfxlib Signed-off-by: Kristoffer Rist Skøien <kristoffer.skoien@nordicsemi.no>

koffes force-pushed the move_lc3 branch from 1ffecb7 to b850bc2 Compare September 8, 2022 09:45

carlescufi approved these changes Sep 8, 2022

View reviewed changes

greg-fer requested a review from b-gent as a code owner September 8, 2022 12:33

github-actions bot added the doc-required PR must not be merged without tech writer approval. label Sep 8, 2022

greg-fer force-pushed the move_lc3 branch from 375dfd7 to 83e967f Compare September 8, 2022 12:35

umapraseeda reviewed Sep 8, 2022

View reviewed changes

lc3/README.rst Show resolved Hide resolved

greg-fer force-pushed the move_lc3 branch from 83e967f to 1f730e2 Compare September 8, 2022 12:56

umapraseeda reviewed Sep 8, 2022

View reviewed changes

koffes commented Sep 8, 2022

View reviewed changes

doc: lc3: doc review

c30bd93

Created a readme file with changelog included. Moved the API PDF to FTP and included link in the readme. Moved legal files to top folder. Deleted original doc files. Signed-off-by: Grzegorz Ferenc <Grzegorz.Ferenc@nordicsemi.no>

greg-fer force-pushed the move_lc3 branch from 1f730e2 to c30bd93 Compare September 8, 2022 13:31

greg-fer approved these changes Sep 8, 2022

View reviewed changes

greg-fer mentioned this pull request Sep 8, 2022

doc: add 2.1.0 release doc updates nrfconnect/sdk-nrf#8510

Merged

14 tasks

carlescufi merged commit 9a6637e into nrfconnect:main Sep 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lc3: Moved LC3 #826

lc3: Moved LC3 #826

koffes commented Sep 7, 2022

CLAassistant commented Sep 7, 2022 •

edited

Loading

erikrobstad Sep 8, 2022

erikrobstad Sep 8, 2022

erikrobstad Sep 8, 2022

greg-fer left a comment •

edited

Loading

rlubos commented Sep 8, 2022

b-gent commented Sep 8, 2022

carlescufi commented Sep 8, 2022

tejlmand left a comment

rlubos commented Sep 8, 2022

carlescufi commented Sep 8, 2022 •

edited

Loading

koffes commented Sep 8, 2022

umapraseeda Sep 8, 2022

umapraseeda Sep 8, 2022

greg-fer Sep 8, 2022

umapraseeda Sep 8, 2022

greg-fer Sep 8, 2022

koffes left a comment

koffes Sep 8, 2022

greg-fer Sep 8, 2022

		@@ -0,0 +1,115 @@
		#
		# Copyright (c) 2022 Nordic Semiconductor ASA

	* This could lead to higher heap consumtion, but is able to manipulate
	* This could lead to higher heap consumption, but is able to manipulate

	* the codec allocates the worse case shared memory required based on the
	* the codec allocates the worst case shared memory required based on the

	* This function closes the decodeHandle's session and and releases allocated session memory.
	* This function closes the decodeHandle's session and releases allocated session memory.

lc3: Moved LC3 #826

lc3: Moved LC3 #826

Conversation

koffes commented Sep 7, 2022

CLAassistant commented Sep 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg-fer left a comment • edited Loading

Choose a reason for hiding this comment

rlubos commented Sep 8, 2022

b-gent commented Sep 8, 2022

carlescufi commented Sep 8, 2022

tejlmand left a comment

Choose a reason for hiding this comment

rlubos commented Sep 8, 2022

carlescufi commented Sep 8, 2022 • edited Loading

koffes commented Sep 8, 2022

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

koffes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Sep 7, 2022 •

edited

Loading

greg-fer left a comment •

edited

Loading

carlescufi commented Sep 8, 2022 •

edited

Loading