Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

applications: nrf5340_audio: SD card playback PR #11910

Merged
merged 1 commit into from
Nov 15, 2023
Merged

applications: nrf5340_audio: SD card playback PR #11910

merged 1 commit into from
Nov 15, 2023

Conversation

MariusVaardal
Copy link

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jul 31, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jul 31, 2023
@NordicBuilder
Copy link
Contributor

Thank you for your contribution!
It seems you are not a member of the nrfconnect GitHub organization. External contributions are handled as follows:
Large contributions, affecting multiple subsystems for example, may be rejected if they are complex, may introduce regressions due to lack of test coverage, or if they are not consistent with the architecture of nRF Connect SDK.
PRs will be run in our continuous integration (CI) test system.
If CI passes, PRs will be tagged for review and merged on successful completion of review. You may be asked to make some modifications to your contribution during review.
If CI fails, PRs may be rejected or may be tagged for review and rework.
PRs that become outdated due to other changes in the repository may be rejected or rework requested.
External contributions will be prioritized for review based on the relevance to current development efforts in nRF Connect SDK. Bug fix PRs will be prioritized.
You may raise issues or ask for help from our Technical Support team by visiting https://devzone.nordicsemi.com/.

Note: This comment is automatically posted and updated by the Contribs GitHub Action.

@NordicBuilder NordicBuilder added the external External contribution label Jul 31, 2023
@MariusVaardal MariusVaardal force-pushed the lc3_playback_draft_pr branch 3 times, most recently from 322c87e to 0e5582f Compare July 31, 2023 11:10
@MariusVaardal MariusVaardal marked this pull request as ready for review July 31, 2023 14:56
@MariusVaardal MariusVaardal force-pushed the lc3_playback_draft_pr branch 3 times, most recently from 5f75add to 4051b3d Compare August 1, 2023 11:41
@MariusVaardal MariusVaardal changed the title Lc3 playback draft pr SD card playback PR Aug 2, 2023
@MariusVaardal MariusVaardal force-pushed the lc3_playback_draft_pr branch 2 times, most recently from b7f1430 to 85fd2af Compare August 3, 2023 07:21
@MariusVaardal MariusVaardal force-pushed the lc3_playback_draft_pr branch 5 times, most recently from 9171a37 to 31209f1 Compare August 3, 2023 15:12
rick1082
rick1082 requested changes Aug 4, 2023
View reviewed changes
Copy link
Contributor

@rick1082 rick1082 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement :) I leave couple comments, please check.

applications/nrf5340_audio/src/modules/lc3_playback.c Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/modules/sd_card_playback.h Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/modules/sd_card_playback.h Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/modules/sd_card_playback.h Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/audio/sw_codec_select.h Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/modules/sd_card_playback.c Outdated Show resolved Hide resolved
@alexsven alexsven removed the request for review from tejlmand August 4, 2023 08:16
alexsven
alexsven requested changes Aug 4, 2023
View reviewed changes
Copy link
Contributor

@alexsven alexsven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not done yet, but here are some comments

applications/nrf5340_audio/prj.conf Outdated Show resolved Hide resolved
applications/nrf5340_audio/prj.conf Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/audio/audio_datapath.c Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/audio/audio_system.c Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/audio/audio_datapath.c Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/modules/sd_card_playback.c Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/modules/sd_card_playback.c Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/modules/sd_card_playback.c Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/modules/sd_card_playback.c Outdated Show resolved Hide resolved
applications/nrf5340_audio/src/modules/sd_card_playback.c Outdated Show resolved Hide resolved
@koffes koffes added the CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. label Aug 4, 2023
@github-actions github-actions bot added CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. and removed CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. labels Nov 9, 2023
@github-actions github-actions bot added CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. and removed CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. labels Nov 9, 2023
greg-fer
greg-fer reviewed Nov 9, 2023
View reviewed changes
applications/nrf5340_audio/src/modules/sd_card_playback.h Outdated
Comment on lines 57 to 63
* @param[in] size_t Size of the input buffer.
*
* @retval 0 Success.
* @retval -EACCES SD card playback is not active.
* @retval Otherwise, error from underlying drivers.
*/
int sd_card_playback_mix_with_stream(void *const pcm_a, size_t pcm_a_size);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a code expert, but should the param[in] mention pcm_a_size and size_t be a *size_t instead?
This is related to the doc build issue.

@github-actions github-actions bot added CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. and removed CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. labels Nov 9, 2023
@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

greg-fer
greg-fer approved these changes Nov 10, 2023
View reviewed changes
Copy link
Contributor

@greg-fer greg-fer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs will be covered in https://nordicsemi.atlassian.net/browse/OCT-2790 in coordination with @andvib

@github-actions github-actions bot added CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. and removed CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. labels Nov 14, 2023
@github-actions github-actions bot added CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. and removed CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. labels Nov 14, 2023
@github-actions github-actions bot added CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. and removed CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. labels Nov 14, 2023
@MariusVaardal MariusVaardal removed the DNM label Nov 14, 2023
@mVaardal
applications: nrf5340_audio: Add lc3_playback module
18f3eb7 
Support for playing .wav and lc3 files from SD card.

Signed-off-by: Marius Vårdal <marius.vardal@nordicsemi.no>
@github-actions github-actions bot added CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. and removed CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. labels Nov 14, 2023
@anangl anangl merged commit f862b2e into nrfconnect:main Nov 15, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erikrobstad erikrobstad erikrobstad approved these changes

@greg-fer greg-fer greg-fer approved these changes

@alexsven alexsven alexsven approved these changes

@rick1082 rick1082 rick1082 approved these changes

@andvib andvib andvib approved these changes

@koffes koffes koffes approved these changes

Assignees
No one assigned
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. external External contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.