Music Module on v4.3

Anyone get the Music module working on netmf 4.3?

I can run the Sine test no problem. When I load a WAV or MP3 I get an infinite loop in DoPlayback() line 366:

 
while (this.dreq.Read())
    Thread.Sleep(1);

this.dreq.Read() is always true, forever. I don’t know enough about what is expected, but if anyone else does, please let me know. Thanks!

Which music module DFRobot or GHI?

GHI. Rev 1.0.

I ran into a problem a while back where EndOfStream was always returning True. This post explains the problem

https://www.ghielectronics.com/community/forum/topic?id=17357

Sprigo,

The issue is not in the file stream - that reads and closes out just fine. this.dreq.Read() is actually a DigitalIO read that is always true. But your tip gave me some ideas.

I compared the 4.2 code with the 4.3 code, and guess what I found?..

v4.2

while (!this.dreq.Read())
    Thread.Sleep(1);

v4.3

while (this.dreq.Read())
    Thread.Sleep(1);

BUG! :wall: Someone flipped the logic here and left out the ‘not’ in 4.3. I put a ! in the code and playback worked without a hang.

However, this alone does not solve the problem completely. What happens is that it seems the last block doesn’t get played, so the end of the audio gets cut off. Not sure how to fix that. Here is the full method, mostly working as I described:

 buffer)
        {
            byte[] block = new byte[32];
            int size = buffer.Length - buffer.Length % 32;

            for (int i = 0; i < size && this.playing; i += 32)
            {
                Array.Copy(buffer, i, block, 0, 32);
                
                while (!this.dreq.Read())
                    Thread.Sleep(1);

                lock (this.spiSyncRoot)
                    this.spiData.Write(block);
            }

            this.OnMusicFinished(this, null);

            this.Reset();

            this.playing = false;
        }

@ dapug - I think njbuch ran into the bug you have found and posted this in codeshare.

https://www.ghielectronics.com/community/codeshare/entry/924

Yes, but only the flipped logic bug. Not sure about the cutoff…

Try another file or format…

I have not touched the Music Module code in a longtime. The for loop is dropping out too early. I am not at my machine to test or have time to do this code properly.

I hate to put this code out as it is ugly and not optimized… at all, but it should get you going on the right track. See if this works for you short term.


for (int i = 0; i <= buffer.Length; i += 32)
{
	if (i + 32 > buffer.Length)
	{
		byte[] block2 = new byte[buffer.Length - size];
		Array.Copy(buffer, i, block2, 0, buffer.Length - size);
		while (!this.dreq.Read())
			Thread.Sleep(1);

		lock (this.spiSyncRoot)
			this.spiData.Write(block2);
	}
	else
	{
		Array.Copy(buffer, i, block, 0, 32);
		while (!this.dreq.Read())
			Thread.Sleep(1);

		lock (this.spiSyncRoot)
			this.spiData.Write(block);
	}
}

Basically if you setup a byte array with length of 100 and reading a block of 32 bytes at a time you will lose the last 4 bytes of the buffer. you need to test for this and write those last bytes. probably best to not close everything down process the buffer like normal and then test for straggler bytes and write if need be, then reset the module playback.

1 Like

Thank you. I don’t understand how this bug has survived…

I think the last array copy needs a little twist with the number of bytes…?

@ dapug - looking forward to a new code share from you :wink:

I think it has to do with how large of a buffer you are sending. if it is really large, the 32 byte block isn’t that much of it and may not be as noticeable. I can write a routine that is a little better once I get more time, if nobody else does it before then. I have just used streams in the past. so I never really used this code.

Ok here is a quick update to the routine that is a little better.


		private void DoPlayback(byte[] buffer)
		{
			byte[] block = new byte[32];
			int size = buffer.Length - buffer.Length % 32;

			for (int i = 0; i < size && this.playing; i += 32)
			{
				Array.Copy(buffer, i, block, 0, 32);

				while (!this.dreq.Read())
					Thread.Sleep(1);

				lock (this.spiSyncRoot)
					this.spiData.Write(block);
			}

			int remainder = buffer.Length % 32;
			if (remainder > 0 && this.playing)
			{
				byte[] block = new byte[remainder];
				Array.Copy(buffer, 0, block, size, remainder);
				while (!this.dreq.Read())
					Thread.Sleep(1);

				lock (this.spiSyncRoot)
					this.spiData.Write(block);
			}

			this.OnMusicFinished(this, null);

			this.Reset();

			this.playing = false;
		}

@ Samurai

I’m now thinking the problem is not related to skipping that last block. Check this out…

With the code as-is (without your change), the WHOLE file plays back IF I simply set a breakpoint before Reset(). Now, if we look in the Reset code:

        private void Reset()
        {
            while (!this.dreq.Read())
                Thread.Sleep(1);

            this.CommandWrite(Music.SCI_MODE, Music.SM_SDINEW | Music.SM_RESET);

            while (!this.dreq.Read())
                Thread.Sleep(1);

            Thread.Sleep(1);

            this.CommandWrite(Music.SCI_CLOCKF, 0xa000);
        }

…we see that there is a check on the IO while (!this.dreq.Read()) to allow whatever remaining playback is needed before next line that writes a RESET. The problem I suspect is that the digital IO is in fact done (will return false, and skips the sleep) but the actual playback is NOT, so we are looping that while{} delay on the WRONG thing.

I’m not sure yet what to check in order to determine playback is complete, but this is clearly wrong.

Bottom Line: Reset() is called too soon, before the playback is complete!

Yea, that reset call was something I was wondering about, but I did not have the module code in front of me. I did test the for though, and it didn’t get all the characters in the small test buffer. But like I said a few bytes isn’t much in the grand scheme of things.

I would think the:


while (!this.dreq.Read())
  Thread.Sleep(1);

would fix this issue as it should waiting until the last write is finished. Maybe a Sleep(1) before the reset call would help?

in regular .NET here is the code I tested the buffer code with:

 
			int arraySize = 100;
			int[] buffer = Enumerable.Range(0, arraySize).ToArray();
			int[] buffer2 = Enumerable.Repeat(0, arraySize).ToArray();
			int[] block = new int[32];
			int size = buffer.Length - buffer.Length % 32;

			for (int i = 0; i < size; i += 32)
			{
				Array.Copy(buffer, i, block, 0, 32);
				Array.Copy(block, 0, buffer2, i, 32);
			}

I did throw that together, so maybe I missed something, but the buffer2 was missing the last few bytes from the first one.

I would have to arbitrarily Sleep(1200) before reset, because I am missing 800-1200ms of actual playback - thats how prematurely Reset() is getting called. But as you can imagine, the remaining/last buffer could be large or small depending on each file or stream pushed through here, so waiting on nothing is dumb.

I can confirm that this code which is (inside the Reset() method)…

while (!this.dreq.Read())
  Thread.Sleep(1);

…is always true, before [em]and[/em] after playback is complete, which means sleep never gets called, thus reset never waits for anything, just plows ahead and forces a reset.

I’m in unfamiliar territory, but just looking at some of the code here, the blocks are written to SPI (after which I dunno where it goes) and the actual audio out is handled elsewhere, apparently on a different thread? I dunno. It just seems we aren’t accessing (or dont have access to) the actual data or object that is aware of “speakers are in use still” - which is what we ought to be waiting on before reset.

That would definitely be the main problem. Could the setup be on the wrong pin from translating the driver over the years? Just throwing out ideas.

@ dapug: I was wondering, did you ever fix the issue with the music module on v4.3?

I can’t claim to be super familiar with how the driver works, only somewhat familiar with the general concepts. So I haven’t gotten any further. I wont have time to reverse engineer it in the next several weeks but if I hear any more suggestions on what specifically to try, I can give it a go and report back.