From 81aa8d0f68bffa67932900df6558cb407fb27ee8 Mon Sep 17 00:00:00 2001 From: IllusionMan1212 Date: Mon, 27 Jan 2025 23:57:57 +0200 Subject: [PATCH] fix: don't hold onto previously decoded frames' samples to reduce mem usage - migrate to os2 - check if the streaminfo min and max block size are within range - use a tracking allocator for the example program --- TODO.md | 10 +--------- example/main_linux.odin | 28 ++++++++++++++++++++++++---- flac.odin | 36 +++++++++++++++++++++++++----------- internal.odin | 1 + 4 files changed, 51 insertions(+), 24 deletions(-) diff --git a/TODO.md b/TODO.md index aca50fa..c2f2555 100644 --- a/TODO.md +++ b/TODO.md @@ -13,15 +13,6 @@ uncommon/11 -> We error with Invalid Signature faulty/01 -> This one works when it shouldn't (Wrong max blocksize) faulty/02 -> This one works when it shouldn't (Wrong max framesize) -faulty/04 -> This one works when it shouldn't. This one has a wrong number of channels (whatever that means) - Each frame has to be played by itself, otherwise plays sped up. -faulty/05 -> This one works when it shouldn't. (Wrong number of samples) - This should be fine to decode and play. -faulty/08 -> This one works when it shouldn't. (Blocksize 65536, exceeds the allowed max of 65535) - We should error here. We segfault currently. -faulty/09 -> This one works when it shouldn't. (Blocksize 1, below the allowed min of 16 (only the last frame is allowed to have less than 16)) - We should error here. We play correctly currently. -faulty/11 -> We return an error now, but we should just warn and decode the audio fine. Things:- [ ] Options @@ -33,6 +24,7 @@ Things:- [ ] Lots of checks (look at the TODOs in flac.odin) [x] Memory optimizations [ ] Speed optimizations + - Does this matter? So far I haven't found a FLAC file that lags while playing or stalls the audio device even without `-o:speed` [ ] Seeking [ ] Fuzzing to test the stability of the code [x] Check if the use-after-free bug happens on Windows, and if so, if the address sanitizer will show us the full stacktrace diff --git a/example/main_linux.odin b/example/main_linux.odin index 540523d..f9f4b0d 100644 --- a/example/main_linux.odin +++ b/example/main_linux.odin @@ -33,15 +33,19 @@ main :: proc() { fmt.eprintln("Failed to alloc memory") return } - allocator := virtual.arena_allocator(&arena) + //allocator := virtual.arena_allocator(&arena) + + track: mem.Tracking_Allocator + mem.tracking_allocator_init(&track, context.allocator) + defer mem.tracking_allocator_destroy(&track) + context.allocator = mem.tracking_allocator(&track) start := time.now() - flac_data, reader, err := flac.load_from_file(os.args[1], allocator) + flac_data, reader, err := flac.load_from_file(os.args[1], context.allocator) if err != nil { fmt.println("Error while reading flac file:", err) os.exit(1) } - defer flac.destroy(flac_data, reader, allocator) fmt.println(flac_data) @@ -57,7 +61,7 @@ main :: proc() { md5_ctx: md5.Context md5.init(&md5_ctx) for { - frame, err := flac.read_next_frame(reader, flac_data, allocator) + frame, err := flac.decode_next_frame(reader, flac_data, context.allocator) if err != nil { if err == .EOF { break @@ -123,4 +127,20 @@ main :: proc() { elapsed := time.diff(start, end) fmt.println("Decoding took", elapsed) + + flac.destroy(flac_data, reader, context.allocator) + + for _, leak in track.allocation_map { + fmt.printfln("%v leaked %m", leak.location, leak.size) + } + + for bad_free in track.bad_free_array { + fmt.printfln("%v allocation %p was freed badly", bad_free.location, bad_free.memory) + } + + fmt.printfln("Peak Mem: %m", track.peak_memory_allocated) + fmt.printfln("Total Mem: %m", track.total_memory_allocated) + fmt.printfln("Current Mem: %m", track.current_memory_allocated) + fmt.printfln("Total Allocs: %v, Total Frees: %v:", track.total_allocation_count, track.total_free_count) + fmt.printfln("Total Mem Free'd: %m", track.total_memory_freed) } diff --git a/flac.odin b/flac.odin index 4460074..f49732f 100644 --- a/flac.odin +++ b/flac.odin @@ -6,7 +6,7 @@ import "core:crypto/legacy/md5" import "core:fmt" import "core:io" import "core:mem" -import "core:os" +import "core:os/os2" import "core:time" Error :: union { @@ -69,8 +69,7 @@ PictureType :: enum u32 { Flac :: struct { metadata: FlacMetadata, - // This holds the samples up to the currently decoded frame. If all frames have - // been decoded then this will hold all the decoded samples. + // This holds the samples of the current frame. samples: [dynamic]i32, } @@ -94,14 +93,15 @@ Picture :: struct { height: u32, depth: u32, num_colors: u32, - data: []byte, + data: []byte `fmt:"-"`, } Frame :: struct { channels: u8, sample_rate: u32, - // A slice of `Flac`'s `samples` field. + // A slice view of `Flac`'s `samples` field. // Contains the frame's decoded samples. + // DO NOT FREE. samples: []i32, } @@ -436,6 +436,12 @@ read_metadata :: proc(r: ^Reader, allocator := context.allocator) -> (flac: ^Fla return nil, .Invalid_Signature } + if (header.streaminfo.min_block_size < 16 || header.streaminfo.min_block_size > 65535) || + (header.streaminfo.max_block_size < 16 || header.streaminfo.max_block_size > 65535) || + (header.streaminfo.min_block_size > header.streaminfo.max_block_size) { + return nil, .Invalid_Block_Size + } + sample_rate := header.streaminfo.sr_chan_bps_ts >> 44 num_channel_minus_one := (header.streaminfo.sr_chan_bps_ts >> 41) & 7 bits_per_sample_minus_one := (header.streaminfo.sr_chan_bps_ts >> 36) & 0x1F @@ -583,7 +589,7 @@ read_metadata :: proc(r: ^Reader, allocator := context.allocator) -> (flac: ^Fla return flac, nil } -read_next_frame :: proc(r: ^Reader, flac: ^Flac, allocator := context.allocator) -> (frame: Frame, err: Error) { +decode_next_frame :: proc(r: ^Reader, flac: ^Flac, allocator := context.allocator) -> (frame: Frame, err: Error) { // // Frame Header // @@ -652,6 +658,14 @@ read_next_frame :: proc(r: ^Reader, flac: ^Flac, allocator := context.allocator) return {}, .Invalid_Block_Size } + // TODO: Any frame but the last one MUST have a block size equal to or greater + // than the minimum block size and MUST have a block size equal to or + // less than the maximum block size. The last frame MUST have a block + // size equal to or less than the maximum block size; it does not have + // to comply to the minimum block size because the block size of that + // frame must be able to accommodate the length of the audio data the + // stream contains + // //if actual_block_size < 15 && !last_frame { // TODO: block sizes less than 16 are only valid for the last frame and MUST NOT be used for any other frame. //} @@ -780,7 +794,7 @@ read_next_frame :: proc(r: ^Reader, flac: ^Flac, allocator := context.allocator) correlate(subframes, channel_assignment) - start := len(flac.samples) + clear(&flac.samples) // Write the samples interleaved. for i in 0.. Error { load_from_file :: proc(filename: string, allocator := context.allocator) -> (flac: ^Flac, r: ^Reader, err: Error) { context.allocator = allocator - file, open_err := os.open(filename) - if open_err != os.ERROR_NONE { + file, open_err := os2.open(filename) + if open_err != nil { fmt.eprintln(open_err) return nil, {}, .Unable_To_Read_File } br := new(bufio.Reader) - bufio.reader_init(br, os.stream_from_handle(file)) + bufio.reader_init(br, os2.to_stream(file)) r = new(Reader) r.r = bufio.reader_to_stream(br) diff --git a/internal.odin b/internal.odin index 3520433..829baf4 100644 --- a/internal.odin +++ b/internal.odin @@ -240,6 +240,7 @@ CueSheetTrackIndex :: struct #packed { reserved: [3]byte, } +@(optimization_mode="favor_size") decode_extended_utf8 :: proc(r: ^Reader) -> (decoded_num: u64, err: Error) { MASKX :: 0b0011_1111 MASK2 :: 0b0001_1111