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

I2s input API and examples #4539

Merged

earlephilhower merged 17 commits into esp8266:master from earlephilhower:i2s_input

Apr 2, 2018

Collaborator

earlephilhower commented Mar 21, 2018 •

edited

Loading

-edit- Ready for testing/review.

Runs well enough to send I2S recorded data to a PC and play over its speakers. Pretty cool!

Rewrite the I2S utils to support stereo I2S microphone inputs.

Add API calls:

i2s_rxtx_begin(bool rx, bool tx);
bool i2s_read_sample(int16_t *l, int16_t *r, bool blocking);

See issue #4496 for the trail.


          Migrate to i2s_struct_t to make I2S RX simpler

a2d93a2

Much of the I2S TX infrastructure can be used to handle I2S input,
so move the per-channel data structures to a struct for easier
changes.

earlephilhower mentioned this pull request

Support I2S audio input #4496

Closed

Collaborator Author

earlephilhower commented Mar 21, 2018 •

edited

Loading

Looks like the new code is working to some degree. Hooking up a real I2S mike and graphing the i2s_read_sample(l,r,nb) gives:


          Preliminary I2S reception shows life

740e9bd

Initial testing of I2S reception API
Add API calls:
i2s_rxtx_begin(bool rx, rool tx);
i2s_read_sample(uint32_t *l, uint32_t *r);

earlephilhower force-pushed the i2s_input branch from 8d03c53 to 740e9bd Compare

March 22, 2018 02:07

earlephilhower added 2 commits

March 21, 2018 19:43


          Refactor i2s_state_t member names, remove i2s_ prefix

de42ee8


          Add simple I2S input example code

0478d47

devyte requested changes

View reviewed changes

cores/esp8266/core_esp8266_i2s.c Outdated

+                uint32_t *       curr_slc_buf; // Current buffer for writing
+                uint32_t         curr_slc_buf_pos; // Position in the current buffer
+                void             (*callback) (void);
+                // Callback function should be defined as 'void ICACHE_FLASH_ATTR function_name()',

Collaborator

devyte Mar 22, 2018

Can the callback really be in Flash? Shouldn't this read "... defined as void ICACHE_RAM_ATTR function_name()"?

Collaborator Author

earlephilhower Mar 22, 2018

You may have hit upon the cause of some random I2S crashes I've seen at idle. The decorator comes from the original author (maybe cut-n-pasted from the mp3 example at some point?). I just mirrored what was there in the new functions. I'll go over and adjust and remove as needed before finishing.

Collaborator Author

earlephilhower Mar 22, 2018

The actual I2S interrupt handler is defined "ICACHE_FLASHATTR" in master!

cores/esp8266/core_esp8266_i2s.c Outdated

		#define I2SI_WS 14


		static bool ICACHE_FLASH_ATTR _i2s_is_full(const i2s_state_t *ch) {

Collaborator

devyte Mar 22, 2018

why do all of these have the ICACHE_FLASH_ATTR decorator, when that is our default?

cores/esp8266/core_esp8266_i2s.c Outdated

+              static bool ICACHE_FLASH_ATTR _i2s_is_full(const i2s_state_t *ch) {
+                if (!ch) {
+                  return false;
+                } else {

Collaborator

devyte Mar 22, 2018

remove else, no need for it. Just let the flow get to the following return statement.

Collaborator Author

earlephilhower Mar 22, 2018

Done.

cores/esp8266/core_esp8266_i2s.c Outdated

+              static bool ICACHE_FLASH_ATTR _i2s_is_empty(const i2s_state_t *ch) {
+                if (!ch) {
+                  return false;
+                } else {

Collaborator

devyte Mar 22, 2018

Remove else.

Collaborator Author

earlephilhower Mar 22, 2018

done

cores/esp8266/core_esp8266_i2s.c Outdated

+              static int16_t ICACHE_FLASH_ATTR _i2s_available(const i2s_state_t *ch) {
+                if (!ch) {
+                  return 0;
+                } else {

Collaborator

devyte Mar 22, 2018

Remove else.

Collaborator Author

earlephilhower Mar 22, 2018

done

cores/esp8266/core_esp8266_i2s.c Outdated

    
                I2SC &= ~(I2STSM | (I2SBMM << I2SBM) | (I2SBDM << I2SBD) | (I2SCDM << I2SCD));

                I2SC |= I2SRF | I2SMR | I2SRSM | I2SRMS | ((sbd_div_best) << I2SBD) | ((scd_div_best) << I2SCD);

                I2SC &= ~(I2STSM | I2SRSM | /*(I2SBMM << I2SBM) |*/ (I2SBDM << I2SBD) | (I2SCDM << I2SCD));

                I2SC |= I2SRF | I2SMR | I2SRMS | ((sbd_div_best) << I2SBD) | ((scd_div_best) << I2SCD);

Collaborator

devyte Mar 22, 2018

What does the change in this line mean? (add comment)

cores/esp8266/core_esp8266_i2s.c Outdated

               }
               void ICACHE_FLASH_ATTR i2s_set_dividers(uint8_t div1, uint8_t div2){
                 div1 &= I2SBDM;
                 div2 &= I2SCDM;
-                I2SC &= ~(I2STSM | (I2SBMM << I2SBM) | (I2SBDM << I2SBD) | (I2SCDM << I2SCD));
+                I2SC &= ~(I2STSM | I2SRSM | (I2SBMM << I2SBM) | (I2SBDM << I2SBD) | (I2SCDM << I2SCD));

Collaborator

devyte Mar 22, 2018

what does the change in this line mean? (add comment)

cores/esp8266/core_esp8266_i2s.c Outdated

    
                SLCC0 &= ~(SLCMM << SLCM); // Clear DMA MODE

                SLCC0 |= (1 << SLCM); // Set DMA MODE to 1

                SLCRXDC |= SLCBINR | SLCBTNR; // Enable INFOR_NO_REPLACE and TOKEN_NO_REPLACE

                SLCRXDC &= ~(/*SLCBRXFE |*/ SLCBRXEM | SLCBRXFM); //disable RX_FILL, RX_EOF_MODE and RX_FILL_MODE

                //Feed DMA the 1st buffer desc addr

Collaborator

devyte Mar 22, 2018

Should this comment be updated to better reflect the new logic below?

cores/esp8266/core_esp8266_i2s.c Outdated

+                  if (!tx) {
+                    return; // OOM Error!
+                  }
+                  pinMode(2, FUNCTION_1);  // I2SO_WS (LRCK)

Collaborator

devyte Mar 22, 2018

These pin numbers should probably be #defined like the I2SI_lol ones

Collaborator Author

earlephilhower Mar 22, 2018

done

cores/esp8266/core_esp8266_i2s.c Outdated

                 i2s_slc_end();
+                if (tx) {
+                  pinMode(2, INPUT);

Collaborator

devyte Mar 22, 2018

#define pin numbers above

Collaborator Author

earlephilhower Mar 22, 2018

done

Collaborator

devyte commented Mar 22, 2018

Overall, looks pretty good, especially if it works!

Collaborator Author

earlephilhower commented Mar 22, 2018

Sorry for the ton of updates. There's no way to consolidate responses into a single one. The ICACHE_FLASH_ATTR was a good catch, and may be a real bug in the existing ISR. The rest, I'm still hacking away and will clean things up. I've built DMA and serial units in Verilog and programmed them on many different systems, but always w/a good block diagram and register reference. W/O those for the 8266, it's a lot more challenging.

Collaborator Author

earlephilhower commented Mar 22, 2018

Oh, and stereo input seems to be working too. This is the demo app running on a hacked AIY voice hat:

earlephilhower added 3 commits

March 22, 2018 07:27


          Address @devyte's style and code issues

bdef111


          Note IRQ potential race condition

d2af6fa


          Fix WDT resets in sample push/pop

4472df0

est_wdt_(dis/en)able were being called instead of yield in the
i2s_(read/write)_sample branches, but those calls don't actually
reset the WDT.  Replace with optimistic_yield(10ms).

Collaborator

devyte commented Mar 23, 2018

@earlephilhower does it make sense to add some unit tests for this?

Collaborator Author

earlephilhower commented Mar 23, 2018

I'll keep that in mind, but because this is running and capturing data via an external pin I'm not sure what can actually be tested. If there was a loopback mode, that would be ideal. Send data on one iface, read it back on the other, compare.


          Merge branch 'master' into i2s_input

35fd325

Collaborator

devyte commented Mar 28, 2018

@earlephilhower this has conflicts now

Collaborator Author

earlephilhower commented Mar 28, 2018

Yup, I figured it would get borked with the latest bugfix. I'll see if I can untangle it.

earlephilhower added 4 commits

March 27, 2018 19:59


          Merge branch 'master' of https://github.com/esp8266/Arduino into i2s_…

f2eabd6

…input


          Correct I2S receive mode to be master

1285d9d

Fix mistake in merge.  I2S recevive was in slave mode (external clock
driven).


          Add intro usage text to example

bc743d0

Mostly for my edification, list the IO pins and where to find them on
the D1 mini and other boards, and how to use the AIY Voice Hat.


          Move to 16bps per channel like xmit, clean up

0039dbc

Some I2S configuration and DMA tweaks makes 16 bits per sample per
channel work, giving a basically identical interface as xmit.

The received data values now span the full range of +/-32767 as well.

earlephilhower changed the title ~~WIP - I2s input API~~ I2s input API and examples

Collaborator Author

earlephilhower commented Mar 28, 2018 •

edited

Loading

I've tested and added an example I2S microphone send-over-UDP example. Runs pretty well if I do say so!

At this stage ready for review and testing by those in the original issue.


          Add I2S input and transmit over WiFi example

81f7f21

Very simple example sends I2S microphone data using UDP over WiFi.
Runs very well and proves the I2S receive DMA buffer management is
working pretty well.

earlephilhower force-pushed the i2s_input branch from f07f361 to 81f7f21 Compare

March 28, 2018 04:39

devyte approved these changes

View reviewed changes

devyte and others added 2 commits

March 28, 2018 09:28


          Merge branch 'master' into i2s_input

8f82ea3


          Fix example formatting, ugh.

ea0699c

earlephilhower force-pushed the i2s_input branch from 08c403b to ea0699c Compare

March 28, 2018 14:24

earlephilhower added 2 commits

March 28, 2018 13:01


          Add better OOM error checking, formatting

c86413b

Clean up a few dangling 1-line-ifs, make the new I2S code check for
an OOM error and return it up to the app instead of crashing when it
tries to memset a NULL pointer.  Old comments updated to new structure.


          Merge branch 'master' into i2s_input

257ec3e

earlephilhower merged commit 8ae553d into esp8266:master

d-a-v reviewed

View reviewed changes

libraries/esp8266/examples/I2STransmit/I2STransmit.ino

+              void loop() {
+                static int cnt = 0;
+                // Each loop will send 100 raw samples (400 bytes)
+                // UDP needs to be < TCP_MSS which can be 500 bytes in LWIP2

Collaborator

d-a-v Apr 2, 2018

I come after the battle with saying that I'm not sure MSS applies to UDP.
UDP is only limited by MTU which is always 1460 on ESP8266 (defined and given by PHY).

Collaborator Author

earlephilhower Apr 2, 2018

Are you sure about that? I started with ~1024 bytes per packet, but no UDP packets ever made it to the PC with the low-mem LWIP. At high-mem it did work. Dropping to 400 bytes or so made things work under both so I left it there. It's also better for I2S latency, since you want to minimize the amount of buffering, anyway.

Collaborator

d-a-v Apr 2, 2018

Yes, I'm sure that it should work, and I'm now sure I have to check why it does not :)

d-a-v mentioned this pull request

check UDP d-a-v/esp82xx-nonos-linklayer#14

Closed

earlephilhower deleted the i2s_input branch

September 30, 2018 17:47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet