Skip to content

Commit

Permalink
midi: fixes for PR comments
Browse files Browse the repository at this point in the history 
- Removed handling for non-MIDI chunks (cf. #1004 (comment))
- Removed redundant chunk length asserts (cf. #1004 (comment))
- Removed redundant header FieldArray (cf. #1004 (comment))
- Replaced FieldUintFn with FieldUnn for SMTPEOffsetEvent (cf. #1004 (comment))
- Added note to midi.md re. supported chunks
  • Loading branch information
@twystd
twystd committed Sep 4, 2024
1 parent 17bac77 commit 9f057b6
Show file tree
Hide file tree
Showing 17 changed files with 208 additions and 270 deletions.
10 changes: 2 additions & 8 deletions format/midi/metaevents.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,8 @@ func decodeSMPTEOffset(d *decode.D) {
d.FieldUintFn("length", vlq, d.UintRequire(5))

d.FieldStruct("smpte_offset", func(d *decode.D) {
d.FieldUintFn("framerate", func(d *decode.D) uint64 {
return d.UintBits(2)
}, framerates)

d.FieldUintFn("hour", func(d *decode.D) uint64 {
return d.UintBits(6)
})

d.FieldU2("framerate", framerates)
d.FieldU6("hour")
d.FieldU8("minute")
d.FieldU8("second")
d.FieldU8("frames")
Expand Down
63 changes: 18 additions & 45 deletions format/midi/midi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package midi
import (
"bytes"
"embed"
"encoding/binary"
"fmt"

"github.com/wader/fq/format"
Expand Down Expand Up @@ -43,11 +42,7 @@ func decodeMIDI(d *decode.D) any {
// ... decode tracks
d.FieldArray("tracks", func(d *decode.D) {
for d.BitsLeft() > 0 {
if err := skipTo(d, "MTrk"); err != nil {
d.Errorf("%v", err)
} else {
d.FieldStruct("track", decodeMTrk)
}
d.FieldStruct("track", decodeMTrk)
}
})

Expand All @@ -59,31 +54,27 @@ func decodeMThd(d *decode.D) {
d.Errorf("no MThd marker")
}

d.FieldArray("header", func(d *decode.D) {
d.FieldUTF8("tag", 4)
length := d.FieldS32("length")

d.AssertLeastBytesLeft(length)
d.FieldUTF8("tag", 4)
length := d.FieldS32("length")

d.FramedFn(length*8, func(d *decode.D) {
format := d.FieldU16("format")
if format != 0 && format != 1 && format != 2 {
d.Errorf("invalid MThd format %v (expected 0,1 or 2)", format)
}
d.FramedFn(length*8, func(d *decode.D) {
format := d.FieldU16("format")
if format != 0 && format != 1 && format != 2 {
d.Errorf("invalid MThd format %v (expected 0,1 or 2)", format)
}

tracks := d.FieldU16("tracks")
if format == 0 && tracks > 1 {
d.Errorf("MIDI format 0 expects 1 track (got %v)", tracks)
}
tracks := d.FieldU16("tracks")
if format == 0 && tracks > 1 {
d.Errorf("MIDI format 0 expects 1 track (got %v)", tracks)
}

division := d.FieldU16("divisions")
if division&0x8000 == 0x8000 {
SMPTE := (division & 0xff00) >> 8
if SMPTE != 0xe8 && SMPTE != 0xe7 && SMPTE != 0xe6 && SMPTE != 0xe5 {
d.Errorf("invalid MThd division SMPTE timecode type %02X (expected E8,E7, E6 or E5)", SMPTE)
}
division := d.FieldU16("divisions")
if division&0x8000 == 0x8000 {
SMPTE := (division & 0xff00) >> 8
if SMPTE != 0xe8 && SMPTE != 0xe7 && SMPTE != 0xe6 && SMPTE != 0xe5 {
d.Errorf("invalid MThd division SMPTE timecode type %02X (expected E8,E7, E6 or E5)", SMPTE)
}
})
}
})
}

Expand All @@ -95,8 +86,6 @@ func decodeMTrk(d *decode.D) {
d.FieldUTF8("tag", 4)
length := d.FieldS32("length")

d.AssertLeastBytesLeft(length)

d.FieldArray("events", func(d *decode.D) {
d.FramedFn(length*8, func(d *decode.D) {
ctx := context{
Expand Down Expand Up @@ -124,22 +113,6 @@ func decodeEvent(d *decode.D, ctx *context) {
}
}

func skipTo(d *decode.D, tag string) error {
for d.BitsLeft() > 0 {
bytes := d.PeekBytes(8)

if string(bytes[0:4]) == tag {
return nil
} else {
length := 8 + binary.BigEndian.Uint32(bytes[4:])

d.SeekRel(8 * int64(length))
}
}

return fmt.Errorf("missing %v chunk", tag)
}

func peekEvent(d *decode.D) (uint64, uint8, uint8) {
var N int = 1

Expand Down
12 changes: 7 additions & 5 deletions format/midi/midi.md
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
### Notes

1. Only supports the MIDI 1.0 specification.
2. Does only basic validation on the MIDI data.
1. Only supports the MIDI 1.0 MIDI file specification.
2. Only supports _MThd_ and _MTrk_ chunks.
3. Does only basic validation on the MIDI data.

### Sample queries

Expand All @@ -26,10 +27,11 @@ fq -d midi 'grep_by(.event=="note_on") | [.time.tick, .note_on.note] | join(" ")
```

### Authors
- transcriptaze.development@gmail.com
- [transcriptaze](https://github.com/transcriptaze)

### References

1. [The Complete MIDI 1.0 Detailed Specification](https://www.midi.org/specifications/item/the-midi-1-0-specification)
2. [Standard MIDI File (SMF) Format](http://midi.teragonaudio.com/tech/midifile.htm)
3. [MIDI Files Specification](http://www.somascape.org/midi/tech/mfile.html)
2. [Standard MIDI Files](https://midi.org/standard-midi-files)
3. [Standard MIDI File (SMF) Format](http://midi.teragonaudio.com/tech/midifile.htm)
4. [MIDI Files Specification](http://www.somascape.org/midi/tech/mfile.html)
11 changes: 3 additions & 8 deletions format/midi/testdata/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,9 @@ run: build
go run . -d midi dv format/midi/testdata/reference.mid

debug: build
go run . -d midi dv format/midi/testdata/events/note-off.mid
go run . -d midi dv format/midi/testdata/events/note-on.mid
go run . -d midi dv format/midi/testdata/events/controller.mid
go run . -d midi dv format/midi/testdata/events/polyphonic-pressure.mid
go run . -d midi dv format/midi/testdata/events/program-change.mid
go run . -d midi dv format/midi/testdata/events/channel-pressure.mid
go run . -d midi dv format/midi/testdata/events/pitch-bend.mid
go run . -d midi dv format/midi/testdata/midi/invalid-MThd-length.mid
go run . -d midi dv format/midi/testdata/midi/invalid-MTrk-length.mid
go run . -d midi dv format/midi/testdata/events/smpte-offset.mid

test: build
go test ./format -run TestFormats/midi
Expand Down Expand Up @@ -78,5 +74,4 @@ query: build
all: test lint
make -f Makefile test
# go test ./format -run TestFormats/all/all.fqtest
# go run . -d midi dv format/midi/testdata/midi/unknown-chunks.mid
# go run . -d midi dv format/midi/testdata/midi/empty.mid
13 changes: 9 additions & 4 deletions format/midi/testdata/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ Basic MIDI format 2 test file. Contains two tracks, each with only a _track nam
5. _empty.mid_
Empty MIDI file to verify MIDI decoder handles empty files.

6. _unknown_chunks.mid_
MIDI file with non-MIDI chunks interleaved between the _MTrk_ track chunks.

6. _key_signatures.mid_

Test file with all supported MIDI key signatures.
Expand All @@ -32,7 +29,15 @@ Test file with all supported MIDI key signatures.

Test file with all supported MIDI notes.

8. _twinkle.mid_
8. _invalid-MThd-length.mid_

Test file with invalid _MThd_ chunk length.

9. _invalid-MTrk-length.mid_

Test file with invalid _MTrk_ chunk length.

10. _twinkle.mid_

Sample valid MIDI file for the example queries in the help.

Expand Down
11 changes: 5 additions & 6 deletions format/midi/testdata/format-0.fqtest
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
$ fq -d midi dv midi/format-0.mid
|00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f|0123456789abcdef|.{}: midi/format-0.mid (midi) 0x0-0x26 (38)
| | | header{}: 0x0-0xe (14)
| | | header[0:5]: 0x0-0xe (14)
0x00|4d 54 68 64 |MThd | [0]: "MThd" tag 0x0-0x4 (4)
0x00| 00 00 00 06 | .... | [1]: 6 length 0x4-0x8 (4)
0x00| 00 00 | .. | [2]: 0 format 0x8-0xa (2)
0x00| 00 01 | .. | [3]: 1 tracks 0xa-0xc (2)
0x00| 01 e0 | .. | [4]: 480 divisions 0xc-0xe (2)
0x00|4d 54 68 64 |MThd | tag: "MThd" 0x0-0x4 (4)
0x00| 00 00 00 06 | .... | length: 6 0x4-0x8 (4)
0x00| 00 00 | .. | format: 0 0x8-0xa (2)
0x00| 00 01 | .. | tracks: 1 0xa-0xc (2)
0x00| 01 e0 | .. | divisions: 480 0xc-0xe (2)
| | | tracks[0:1]: 0xe-0x26 (24)
| | | [0]{}: track 0xe-0x26 (24)
0x00| 4d 54| MT| tag: "MTrk" 0xe-0x12 (4)
Expand Down
11 changes: 5 additions & 6 deletions format/midi/testdata/format-1.fqtest
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
$ fq -d midi dv midi/format-1.mid
|00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f|0123456789abcdef|.{}: midi/format-1.mid (midi) 0x0-0x45 (69)
| | | header{}: 0x0-0xe (14)
| | | header[0:5]: 0x0-0xe (14)
0x00|4d 54 68 64 |MThd | [0]: "MThd" tag 0x0-0x4 (4)
0x00| 00 00 00 06 | .... | [1]: 6 length 0x4-0x8 (4)
0x00| 00 01 | .. | [2]: 1 format 0x8-0xa (2)
0x00| 00 02 | .. | [3]: 2 tracks 0xa-0xc (2)
0x00| 01 e0 | .. | [4]: 480 divisions 0xc-0xe (2)
0x00|4d 54 68 64 |MThd | tag: "MThd" 0x0-0x4 (4)
0x00| 00 00 00 06 | .... | length: 6 0x4-0x8 (4)
0x00| 00 01 | .. | format: 1 0x8-0xa (2)
0x00| 00 02 | .. | tracks: 2 0xa-0xc (2)
0x00| 01 e0 | .. | divisions: 480 0xc-0xe (2)
| | | tracks[0:2]: 0xe-0x45 (55)
| | | [0]{}: track 0xe-0x26 (24)
0x00| 4d 54| MT| tag: "MTrk" 0xe-0x12 (4)
Expand Down
11 changes: 5 additions & 6 deletions format/midi/testdata/format-2.fqtest
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
$ fq -d midi dv midi/format-2.mid
|00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f|0123456789abcdef|.{}: midi/format-2.mid (midi) 0x0-0x43 (67)
| | | header{}: 0x0-0xe (14)
| | | header[0:5]: 0x0-0xe (14)
0x00|4d 54 68 64 |MThd | [0]: "MThd" tag 0x0-0x4 (4)
0x00| 00 00 00 06 | .... | [1]: 6 length 0x4-0x8 (4)
0x00| 00 02 | .. | [2]: 2 format 0x8-0xa (2)
0x00| 00 02 | .. | [3]: 2 tracks 0xa-0xc (2)
0x00| 01 e0 | .. | [4]: 480 divisions 0xc-0xe (2)
0x00|4d 54 68 64 |MThd | tag: "MThd" 0x0-0x4 (4)
0x00| 00 00 00 06 | .... | length: 6 0x4-0x8 (4)
0x00| 00 02 | .. | format: 2 0x8-0xa (2)
0x00| 00 02 | .. | tracks: 2 0xa-0xc (2)
0x00| 01 e0 | .. | divisions: 480 0xc-0xe (2)
| | | tracks[0:2]: 0xe-0x43 (53)
| | | [0]{}: track 0xe-0x28 (26)
0x00| 4d 54| MT| tag: "MTrk" 0xe-0x12 (4)
Expand Down
6 changes: 4 additions & 2 deletions format/midi/testdata/help_midi.fqtest
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ Decode examples

Notes
=====
* Only supports the MIDI 1.0 specification.
* Only supports the MIDI 1.0 MIDI file specification.
* Only supports and chunks.
* Does only basic validation on the MIDI data.

Sample queries
Expand All @@ -34,10 +35,11 @@ Sample queries

Authors
=======
- transcriptaze.development@gmail.com
- transcriptaze (https://github.com/transcriptaze)

References
==========
* The Complete MIDI 1.0 Detailed Specification (https://www.midi.org/specifications/item/the-midi-1-0-specification)
* Standard MIDI Files (https://midi.org/standard-midi-files)
* Standard MIDI File (SMF) Format (http://midi.teragonaudio.com/tech/midifile.htm)
* MIDI Files Specification (http://www.somascape.org/midi/tech/mfile.html)
11 changes: 5 additions & 6 deletions format/midi/testdata/keys.fqtest
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
$ fq -d midi d midi/key-signatures.mid
|00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f|0123456789abcdef|.{}: midi/key-signatures.mid (midi)
| | | header{}:
| | | header[0:5]:
0x000|4d 54 68 64 |MThd | [0]: "MThd"
0x000| 00 00 00 06 | .... | [1]: 6
0x000| 00 00 | .. | [2]: 0
0x000| 00 01 | .. | [3]: 1
0x000| 01 e0 | .. | [4]: 480
0x000|4d 54 68 64 |MThd | tag: "MThd"
0x000| 00 00 00 06 | .... | length: 6
0x000| 00 00 | .. | format: 0
0x000| 00 01 | .. | tracks: 1
0x000| 01 e0 | .. | divisions: 480
| | | tracks[0:1]:
| | | [0]{}: track
0x000| 4d 54| MT| tag: "MTrk"
Expand Down
Loading

0 comments on commit 9f057b6

Please sign in to comment.