patternMinor
A Virtual Piano
Viewed 0 times
virtualpianostackoverflow
Problem
I have created a virtual piano that turns the home row (excluding 'g' and 'h') and the top row (excluding 'y' and 'u') of the user's keyboard in a piano's keyboard.
The home row is used for white keys and the top row is used for black keys.
The keyboard's keys bind to piano's as shown below:
To change octaves, you use 'z' to go up one octave and 'x' to down one octave.
The code
virtual_piano.asm
```
;===============================================================================
; Virtual Piano -- a virtual and playable piano
; By SirPython of Code Review and GitHub
;
; virtual_piano.asm
;===============================================================================
%define MIDI_CONTROL_PORT 0331h
%define MIDI_DATA_PORT 0330h
%define MIDI_UART_MODE 3Fh
%define MIDI_PIANO_INSTRUMENT 93h
start:
call setup_midi
mov ch, 60; default octave(0)
mov cl, 5; used for making sure that the user does not go too low or too high with the octaves
.loop:
call read_character
call process_input
cmp bh, 0; if bad input OR octave change goes out of range
je .loop
call get_pitch
cmp bh, 2; if shouldn't play note (was an octave switch)
je .loop
call play_note
jmp .loop
;--------------------------------------------------
; Plays a note
;
; IN: AL, CH = pitch, (octave * 12) + 60
; OUT: NONE
; ERR: NONE
; REG: AL
play_note:
add al, ch; apply the octave
out dx, al; DX will already contain MIDI_DATA_PORT from the setup_midi function
mov al, 7Fh; note duration
out dx, al
ret
;--------------------------------------------------
; Based on input, returns a pitch to be played
;
; IN: AL = key code
; OUT: AL, BH, CH = pitch, 2 if no pitch to be played, (octave * 12) + 60
; ERR: NONE
; REG: preserved
get_pitch:
cm
The home row is used for white keys and the top row is used for black keys.
The keyboard's keys bind to piano's as shown below:
a - C
w - C#
s - D
e - D#
d - E
r - E# (F)
f - F
t - F#
j - G
i - G#
k - A
o - A#
l - B
p - B# (C)
; - CTo change octaves, you use 'z' to go up one octave and 'x' to down one octave.
The code
virtual_piano.asm
```
;===============================================================================
; Virtual Piano -- a virtual and playable piano
; By SirPython of Code Review and GitHub
;
; virtual_piano.asm
;===============================================================================
%define MIDI_CONTROL_PORT 0331h
%define MIDI_DATA_PORT 0330h
%define MIDI_UART_MODE 3Fh
%define MIDI_PIANO_INSTRUMENT 93h
start:
call setup_midi
mov ch, 60; default octave(0)
mov cl, 5; used for making sure that the user does not go too low or too high with the octaves
.loop:
call read_character
call process_input
cmp bh, 0; if bad input OR octave change goes out of range
je .loop
call get_pitch
cmp bh, 2; if shouldn't play note (was an octave switch)
je .loop
call play_note
jmp .loop
;--------------------------------------------------
; Plays a note
;
; IN: AL, CH = pitch, (octave * 12) + 60
; OUT: NONE
; ERR: NONE
; REG: AL
play_note:
add al, ch; apply the octave
out dx, al; DX will already contain MIDI_DATA_PORT from the setup_midi function
mov al, 7Fh; note duration
out dx, al
ret
;--------------------------------------------------
; Based on input, returns a pitch to be played
;
; IN: AL = key code
; OUT: AL, BH, CH = pitch, 2 if no pitch to be played, (octave * 12) + 60
; ERR: NONE
; REG: preserved
get_pitch:
cm
Solution
I see a number of things that may help you improve your program.
Commenting
Your per-routine header comments are excellent. They provide exactly the right information that a programmer would want to know about the program. Namely, what registers are expected, which are returned and which are trashed. I have not continued that style within the code I show here, but that's only for the sake of brevity. Please do continue with that style -- it's a very good habit! For a slightly different approach here is an example showing my usual commenting style.
Think of the user
There are a few user-oriented things I think should be improved. First, I see that you have the notes for the keys a through ; arranged from left to right and from low to high, just like a real piano. That's good because it is intuitive to the user. However, in the same way, I would expect z to cause the octave to be lower and x to make it go higher, but it's the other way around in this program. Also, there really ought to be a way to allow the user to exit the program! I would suggest something obvious like the Esc key.
Carefully consider what the program must do
As you have suspected, there are indeed better ways to handle the very large "switch statement" currently in the program. The way I typically write assembly language programs like this is to start with what we have as input (in this case the keycode in the
However, some keys, namely z and x don't emit a sound. They just change the octave. Also, as I've mentioned in the previous item, there should be a way to exit the program. So our modified psuedocode might look like this:
It's still a bit messy. Another way to construct things would be like this:
Now that's looking fairly neat, so let's see how to do that.
Use a data structure
We want to map a keycode to a particular action, so we could have a list of tuples:
Then we can use the data structure to define all key actions:
Then all that's left is to define
Streamline range checking
Currently two 8-bit registers are being used to represent octaves. The
Note that this has the attribute of pegging the value so that if, due to some kind of error, the octave value were out of range, it will always return to being in range after this call.
Avoid overrunning slower hardware
The interface you are using was popularized many years ago by a dominant manufacturer of sound cards then. Documentation for that inte
Commenting
Your per-routine header comments are excellent. They provide exactly the right information that a programmer would want to know about the program. Namely, what registers are expected, which are returned and which are trashed. I have not continued that style within the code I show here, but that's only for the sake of brevity. Please do continue with that style -- it's a very good habit! For a slightly different approach here is an example showing my usual commenting style.
Think of the user
There are a few user-oriented things I think should be improved. First, I see that you have the notes for the keys a through ; arranged from left to right and from low to high, just like a real piano. That's good because it is intuitive to the user. However, in the same way, I would expect z to cause the octave to be lower and x to make it go higher, but it's the other way around in this program. Also, there really ought to be a way to allow the user to exit the program! I would suggest something obvious like the Esc key.
Carefully consider what the program must do
As you have suspected, there are indeed better ways to handle the very large "switch statement" currently in the program. The way I typically write assembly language programs like this is to start with what we have as input (in this case the keycode in the
AL register), and what we ultimately need to output, which is a pitch, also in AL. So in pseudocode it might look like this:- get keycode
- translate AL = code to AL = pitch
- play sound
However, some keys, namely z and x don't emit a sound. They just change the octave. Also, as I've mentioned in the previous item, there should be a way to exit the program. So our modified psuedocode might look like this:
- get keycode
- if it's Esc, quit the program
- if it's z, make the octave lower
- if it's x, make the octave higher
- if it's any other valid key, translate to pitch and play sound
- goto step 1
It's still a bit messy. Another way to construct things would be like this:
- get keycode
- lookup keycode action
- if no action, goto 1
- otherwise execute action
Now that's looking fairly neat, so let's see how to do that.
Use a data structure
We want to map a keycode to a particular action, so we could have a list of tuples:
keycode, subroutine, but the only difference between the action for a and the action for s is a different value for the pitch. Likewise the difference between z and x is only the direction of the octave change. This suggests that we can very efficiently handle all of these with tuples like this: keycode, value, subroutine. In NASM assembly:; this is the structure used for key handling
struc keyhandle
.keycode resb 1
.value resb 1
.process resw 1
endstrucThen we can use the data structure to define all key actions:
keys db 'a',0
dw play_note
db 's',2
dw play_note
db 'd',4
dw play_note
db 'f',5
dw play_note
db 'j',7
dw play_note
db 'k',9
dw play_note
db 'l',11
dw play_note
db ';',12
dw play_note
db 'w',1
dw play_note
db 'e',3
dw play_note
db 'r',5
dw play_note
db 't',6
dw play_note
db 'i',8
dw play_note
db 'o',10
dw play_note
db 'p',11
dw play_note
db 'z',-12
dw change_octave
db 'x',+12
dw change_octave
db 1bh,0
dw quit
; end of list marker
db 0,0
dw 0Then all that's left is to define
play_note, change_octave and quit routines. Since quit is simplest, I show it here:quit:
mov ah,4ch
int 21hStreamline range checking
Currently two 8-bit registers are being used to represent octaves. The
ch register holds the actual octave value that is used to calculate the pitch, and the cl register holds the value representing the octave number. However, both are not really needed. The minimum value is 12, and the maximum usable value is probably 108. The program currently only effectively limits the low end of the range, but fails on the high end. I would propose instead using only ch and having a change_octave routine that accepts the step (+12 or -12) in al:change_octave:
add ch, al
cmp ch, 108
jbe .ok
mov ch, 108
.ok:
cmp ch, 12
jae .done
mov ch, 12
.done:
retNote that this has the attribute of pegging the value so that if, due to some kind of error, the octave value were out of range, it will always return to being in range after this call.
Avoid overrunning slower hardware
The interface you are using was popularized many years ago by a dominant manufacturer of sound cards then. Documentation for that inte
Code Snippets
; this is the structure used for key handling
struc keyhandle
.keycode resb 1
.value resb 1
.process resw 1
endstruckeys db 'a',0
dw play_note
db 's',2
dw play_note
db 'd',4
dw play_note
db 'f',5
dw play_note
db 'j',7
dw play_note
db 'k',9
dw play_note
db 'l',11
dw play_note
db ';',12
dw play_note
db 'w',1
dw play_note
db 'e',3
dw play_note
db 'r',5
dw play_note
db 't',6
dw play_note
db 'i',8
dw play_note
db 'o',10
dw play_note
db 'p',11
dw play_note
db 'z',-12
dw change_octave
db 'x',+12
dw change_octave
db 1bh,0
dw quit
; end of list marker
db 0,0
dw 0quit:
mov ah,4ch
int 21hchange_octave:
add ch, al
cmp ch, 108
jbe .ok
mov ch, 108
.ok:
cmp ch, 12
jae .done
mov ch, 12
.done:
retplay_note:
add al, ch; apply the octave
out dx, al; DX = MIDI_DATA_PORT
inc dx
.busy:
in al, dx
test al,40h ; ready for output?
jnz .busy
dec dx
mov al, 7Fh; note duration
out dx, al
inc dx
.notready:
in al,dx
test al,40h ; ready for output?
jnz .notready
dec dx
retContext
StackExchange Code Review Q#87322, answer score: 7
Revisions (0)
No revisions yet.