You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/04/10 17:27:33 UTC
[GitHub] [incubator-nuttx] okayserh opened a new pull request, #6021: Test/wm8904balance
okayserh opened a new pull request, #6021:
URL: https://github.com/apache/incubator-nuttx/pull/6021
## Summary
I've been working on audio support for the stm32f746 discovery board and have created
an adapted driver for the wm8994.c from wm8904.c. During testing of that code, I've seen
that the balance functionality does not work properly. The range of values for the balance
varied between initialization in wm8904_hw_reset (0..65536) and the
wm8904_setvolume (0..1000) function. The patch corrects this to get a unified range of
(0..65535). This value range was chosen to avoid an overflow at 100% balance.
Furthermore in wm8904_configure the case for setting the balance was added.
## Impact
Should make the "balance" function operational for the wm8904.
## Testing
Due to lack of hardware with the wm8904, the submitted patch has only been tested in a currently
development version of wm8994.c together with the stm32f746 disco. Logic works properly in that
context.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on a diff in pull request #6021: Test/wm8904balance
Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #6021:
URL: https://github.com/apache/incubator-nuttx/pull/6021#discussion_r846850017
##########
drivers/audio/wm8904.c:
##########
@@ -1210,6 +1214,30 @@ static int wm8904_configure(FAR struct audio_lowerhalf_s *dev,
break;
#endif /* CONFIG_AUDIO_EXCLUDE_VOLUME */
+#ifndef CONFIG_AUDIO_EXCLUDE_BALANCE
+ case AUDIO_FU_BALANCE:
+ {
+ /* Set the balance. The percentage level * 10 (0-1000) is in the
+ * ac_controls.b[0] parameter.
+ */
+
+ uint16_t balance = caps->ac_controls.hw[0];
+ audinfo(" Balance: %d\n", balance);
+
+ if (balance >= 0 && balance <= 1000)
Review Comment:
Ok, so let keep it this way :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on a diff in pull request #6021: Test/wm8904balance
Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #6021:
URL: https://github.com/apache/incubator-nuttx/pull/6021#discussion_r846823557
##########
drivers/audio/wm8904.c:
##########
@@ -480,22 +480,24 @@ static void wm8904_setvolume(FAR struct wm8904_dev_s *priv, uint16_t volume,
#ifndef CONFIG_AUDIO_EXCLUDE_BALANCE
/* Calculate the left channel volume level {0..1000} */
Review Comment:
Please fix the comment as well
##########
drivers/audio/wm8904.c:
##########
@@ -1210,6 +1214,30 @@ static int wm8904_configure(FAR struct audio_lowerhalf_s *dev,
break;
#endif /* CONFIG_AUDIO_EXCLUDE_VOLUME */
+#ifndef CONFIG_AUDIO_EXCLUDE_BALANCE
+ case AUDIO_FU_BALANCE:
+ {
+ /* Set the balance. The percentage level * 10 (0-1000) is in the
+ * ac_controls.b[0] parameter.
+ */
+
+ uint16_t balance = caps->ac_controls.hw[0];
+ audinfo(" Balance: %d\n", balance);
+
+ if (balance >= 0 && balance <= 1000)
Review Comment:
Why not to change the application side to use 0-65535 range as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis merged pull request #6021: Test/wm8904balance
Posted by GitBox <gi...@apache.org>.
acassis merged PR #6021:
URL: https://github.com/apache/incubator-nuttx/pull/6021
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] okayserh commented on a diff in pull request #6021: Test/wm8904balance
Posted by GitBox <gi...@apache.org>.
okayserh commented on code in PR #6021:
URL: https://github.com/apache/incubator-nuttx/pull/6021#discussion_r846834189
##########
drivers/audio/wm8904.c:
##########
@@ -1210,6 +1214,30 @@ static int wm8904_configure(FAR struct audio_lowerhalf_s *dev,
break;
#endif /* CONFIG_AUDIO_EXCLUDE_VOLUME */
+#ifndef CONFIG_AUDIO_EXCLUDE_BALANCE
+ case AUDIO_FU_BALANCE:
+ {
+ /* Set the balance. The percentage level * 10 (0-1000) is in the
+ * ac_controls.b[0] parameter.
+ */
+
+ uint16_t balance = caps->ac_controls.hw[0];
+ audinfo(" Balance: %d\n", balance);
+
+ if (balance >= 0 && balance <= 1000)
Review Comment:
Thanks for reviewing! From "apps/system/nxplayer/nxplayer.c" (line 2142) and "apps/system/nxplayer/nxplayer_main.c" (line 120) I deduced that applications seem to assume a range from 0 to 1000 for volume and balance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org