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

Trying to record program from old computer #175

Open
rickyelqasem opened this issue Sep 12, 2022 · 23 comments
Open

Trying to record program from old computer #175

rickyelqasem opened this issue Sep 12, 2022 · 23 comments
Labels
bug

Comments

@rickyelqasem
Copy link

I have a lot of inconsistency while trying to record programs from old computers. If the program is short, the a better chance I will be able to load it later. Playback using my circuit is perfect because if I load a program that was converted to a wav using a converter on my PC, playback works every time. Programs recorded(32000) to wav using this library hardly playback. I've played around with amplification, but I don't think it's that. I think the library is adding artefacts to the sound wave. So for example a good file might play a single flat tone, while if it's recorded using the library, I hear a warble or ringing when there should be a flat tone. I can dropbox you an example of a good/bad file. Any advice to improve? BTW I cannot increase buffsize past 76 due to the lack of memory taken by the rest of the firmware. thx
@TMRh20
Copy link
Owner

TMRh20 commented Sep 12, 2022 via email

@rickyelqasem
Copy link
Author

if I use a sine to square wave converter, do you think I will get a better response? also I just updated SDfat to see if I get better results.

@TMRh20
Copy link
Owner

TMRh20 commented Sep 12, 2022 via email

@rickyelqasem
Copy link
Author

OK I spotted an anomaly. So when I'm recording a square wave every 1 sec or so, I see an anomaly written like a triangle instead of a square. Recording at non-standard frequency like 22025 doesn't make a difference. Is there a parameter to tweak maybe in the setup of the timer?
sqr-tri-sqr

@rickyelqasem
Copy link
Author

Also tested it with a signal generator using a fixed frequency and still got the anomalies. This wave should be uniform.
sig-gen

@TMRh20
Copy link
Owner

TMRh20 commented Oct 5, 2022

Ahh, I'm going to assume this is related to using interrupts to drive everything.

In a blog post a long while back, I did some experimentation with high speed signaling on Arduino, and found that using interrupts, things can be a bit choppy, as far as the timing of things goes. Since we are using nested interrupts here, I'll assume that is the problem.

In that post there is some assembly code that more or less regulates the timing of interrupts, so they occur at the same time, every time. I could possibly be modified to work with TMRpcm, but I have not had time to experiment with it.

static void inline wait_until(uint8_t time) {

__asm__ __volatile__ (
    "sub    %[time], %[tcnt1l]\n\t" //Subtract the low byte of TCNT1 from the wait time
    "brmi   102f\n\t"               //If result is negative, we missed it. Branch to 102
 "100:\n\t"                     
    "subi    %[time], 3\n\t"         //Subtract 3 from time
    "brcc    100b\n\t"               //Loop until we get a negative
    "subi    %[time], 0-3\n\t"       //Add 3 back to our value
    "breq    101f\n\t"               //If equal to 0, branch to 101 and delay 1/16 of a uS (No OPeration)
    "dec    %[time]\n\t"            //Decrement our value by 1
    "breq    102f\n\t"               //If equal to 0, branch to 102 and don't delay
    "rjmp    102f\n"                 //Else Jump to the end
 "101:\n\t"
    "nop\n"
 "102:\n"
     :                                 //Output variables (none)
     : [time] "a" (time),              //Input variables
       [tcnt1l] "a" (TCNT1L)
);
 
}

This would be called from inside one of the interrupts in TMRpcm. If it is fast enough for 8mhz signaling, I assume it would smooth out the output of TMRpcm as well, but again I haven't had time to experiment. You would need to change TCNT1 to the value of one of the timers used in playback/recording of TMRpcm. (I have to review the code to figure this all out) I think you could use TCNT1, but again I have to review the code.

All in all, TMRpcm isn't 100% accurate in its reproduction of audio, but this may smooth things out a bit if it can be gotten working. I'll see if I have time over the next days/week or so to play around with it. Unfortunately I don't have an oscilloscope so can't see the waveforms created.

@rickyelqasem
Copy link
Author

Happy help test. I have a cheap signal generator and an oscilloscope.

@stripwax
Copy link

stripwax commented Oct 8, 2022 •
edited
Loading

(To start: I can reproduce the same problems rickyelqasem reported)

I (think I) see a few problems in the code. But please take all this with a huge grain of salt / open-mindedness just because I'm going thru the code and the datasheet at the same time, and may well be misunderstanding a few things.

Firstly, it looks like it's signalling the wrong buffer to write next (should be bufferEmpty[whichBuffer]=0 , not bufferEmpty[!whichBuffer]=0, in the TIMER1_COMPB_vect). Basically, the first buffer to fill (which is whichBuffer) should be the first buffer to write, but the code signals the other buffer to be written next.

Secondly, I think, the ISR priorities are wrong - the write-to-sd ISR (COMPA) has a higher priority than the read-adc-to-buffer (COMPB), so the write to sd interrupts actually filling the buffer. Probably this is causing lost samples. I think this probably the main issue.

I can kindof see why (the ADC is tied to Timer1 COMPB). Maybe using COMPA is wrong here then, and should just use TIMER1_OVF_vect instead. In other words, keep COMPB for adc, and use a lower priority (rather than a higher priority) ISR for dumping buffers to sd.

As I understand it, you cannot record and playback different things at the same time anyway (there's only one buffer after all), so I don't see why TIMER1_OVF_vect couldn't dual-purpose for playback usage (i.e. what it's currently doing) and recording usage (i.e. the write-to-sd). So just "if recording then.." at the start of the ISR.

Maybe a couple of other problems related to COMPB : the OCR1B isn't ever explicitly initialised in the recording setup, but it is used in the Speaker2 logic, so it could arbitrarily be wrong at this point. (Guessing it should be reset to 0 when you start recording).

I'm not sure I quite understand the TIMER1_CAPT_vect - I guess it's just toggling the enable and then waking itself up repeatedly - neat trick if so! But then wouldn't that also work for recording (as an alternative to the above suggestion) - use the TIMER1_CAPT_vect for dumping buffers to sd in recording mode - in the same way it's being used to read buffers from sd in playback mode?

I haven't tried implementing all the above (I started having a play around swapping the COMPA and COMPB ISRs but I forget to change the OC1E1A for OC1E1B, got into an infinite loop during writing to sd and actually totally trashed two sd cards, so I need to purchase some more first before I can retest!)

I think there might also be a bug in playback (unrelated to this), just from reading the code anyway - I'll open a separate ticket for that

@rickyelqasem
Copy link
Author

rickyelqasem commented Oct 8, 2022 via email

@stripwax
Copy link

stripwax commented Oct 8, 2022 via email

@TMRh20
Copy link
Owner

TMRh20 commented Oct 8, 2022

Heh, nice analysis. I'd have to really review the code and datasheet to see what your analysis means for the code. If you have time to play around with the code and analyze things in more detail, please do. Pull requests are welcome if you are able to put something together that improves functionality! I've developed the library without the use of an oscilloscope, so there may be a few issues here or there that need to be figured out in greater detail.

@stripwax
Copy link

stripwax commented Oct 8, 2022

Yep, I'll try something out. I don't have an oscilloscope either (wouldn't know how to use one even if I did). I'll just get rid of the COMPA_vect (which interrupts the sampling because its higher than COMPB) and use the CAPT or OVF instead (which is lower than COMPB)

@stripwax
Copy link

stripwax commented Oct 9, 2022

After some more experiments I think I've convinced myself that the interrupt priority was a red-herring and wouldn't have a significant impact on fixing this. I don't see (hear) any evidence of dropped samples on the 'rec+thru' mode, which would be the case if writing to sd was somehow blocking sample capture. So I think you can disregard the "Secondly" comments above :-/ The first part does still seem to be a bug, but not a significant one (it seems to lessen the problem, for me anyway, but not remove it). A more significant improvement seems to be from making the buffer bigger.

@TMRh20
Copy link
Owner

TMRh20 commented Oct 9, 2022

"Also tested it with a signal generator using a fixed frequency and still got the anomalies. This wave should be uniform."

@rickyelqasem Do you recall what fixed frequency you were using and what sample rate you were recording at?

@rickyelqasem
Copy link
Author

rickyelqasem commented Oct 9, 2022 via email

@stripwax
Copy link

stripwax commented Oct 9, 2022 via email

@TMRh20
Copy link
Owner

TMRh20 commented Oct 9, 2022 •
edited
Loading

If that's the case, then I'm thinking you're getting dropped samples. The best way to work around this would be to increase the buffer size as @stripwax mentioned, as well as using the SDFat library with potentially a lower recording sample rate. It should lose less samples that way.
In this case, there is probably not much that I can to library-wise to fix it unless we can identify some bugs, but I have a feeling the main issue is the speed of the microcontroller not being able to keep up. If you reduce the sample rate to say 16khz and record a 3khz sample, do you end up with the same inconsistencies or does it help? If it stays the same there may be some buggy behaviour, if it helps I would assume an MCU speed issue.

@rickyelqasem
Copy link
Author

rickyelqasem commented Oct 9, 2022 via email

@TMRh20
Copy link
Owner

TMRh20 commented Oct 9, 2022

If you're getting sample drops at low sample rates, something is probably wrong somewhere. Not sure how to go about addressing this without a scope though... I'll see what I can figure out.

@TMRh20 TMRh20 added the bug label Oct 9, 2022
@rickyelqasem
Copy link
Author

rickyelqasem commented Oct 9, 2022 via email

@stripwax
Copy link

stripwax commented Oct 9, 2022

@rickyelqasem I don't think it's (just) the timers - your first screenshot shows quite a lot of clipping and a dc offset. The periods you're pointing to have different shaped tops and bottoms, true, but that's more likely to be an analogue artifact. Measuring peak-to-peak should be fairly constant. Those screenshots actually look pretty OK.
There could be a problem with timing (as I mentioned earlier, the Compare B counter isn't explicitly set by the startRecording function so could just be arbitrary), but that shouldn't make things variable. (I can't remember whether the recording interrupts trigger on the Compare A counter, which is also used for the PWM. If they do, that would definitely also be a problem!)
But I think you and I have also both found the best way to detect the glitches, which is to listen to the result.

@stripwax
Copy link

stripwax commented Oct 9, 2022 •
edited
Loading

Here's a prototype showing the changes I had in mind, but it's totally untested right now. I'll see if any of this makes any difference in practice. https://github.com/stripwax/TMRpcm/tree/recording_fixes.

(UPDATED) I've tested this and I'm pretty happy with it. With a buffer of 254 I get zero pops/clicks, but I still get problems with a buffer size of 128. However, if I also set the following flag, for SDFat, I get clean output with a buffer size of 128:
-D CHECK_FLASH_PROGRAMMING=0
but I still hear glitches with a buffersize of 64 or below.

Without my changes, I hear an audible blip/glitch in the output around every one or two seconds when buffer size is 128, and worse when buffer size is 64, and setting the CHECK_FLASH_PROGAMMING flag doesn't seem to make a difference.

I'm fairly certain these results are repeatable and reproducible

My setup is just Timer1 with a Arduino Nano (Mega328p), and mono 8 bit. I've not tried/tested any of the USE_TIMER2 / MODE2 / DUAL / STEREO / 16bit combinations, or stuff for other devices, so even though I have those changes in my branch too, those are totally untested. And I've not tested playback/loops/etc, other than through the example sketch playing back what I just recorded.

@TMRh20
Copy link
Owner

TMRh20 commented Oct 10, 2022

@stripwax Wow lots of changes, nice stuff! Not many people that understand timers and interrupts to this degree, and even having written the code it will take me some time to go through it as well as test.
@rickyelqasem Are you able to test these changes as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@TMRh20 @stripwax @rickyelqasem