From c0788027ba8f48f7d94555cbdbc2579e2a34cd53 Mon Sep 17 00:00:00 2001 From: Mitchell Date: Thu, 30 Apr 2026 23:26:42 -0500 Subject: [PATCH] refactor: clean up functions that make allocations There were some functions that returned allocated memory from an internal allocator. This is messy. These types of functions should take an allocator as an argument. --- src/audio/audio_encoder.zig | 48 +++++++++++-------- src/audio/audio_timeline.zig | 8 ++-- .../video/linux/pipewire/pipewire_video.zig | 2 +- src/file_picker/file_picker.zig | 16 +++---- .../linux/xdg_desktop_portal_file_picker.zig | 39 +++++++-------- .../windows/windows_file_picker.zig | 22 +++------ src/main.zig | 8 ++-- src/state/user_settings_state.zig | 2 +- src/vulkan/vulkan.zig | 11 +++-- 9 files changed, 76 insertions(+), 80 deletions(-) diff --git a/src/audio/audio_encoder.zig b/src/audio/audio_encoder.zig index 37c5684..44ec553 100644 --- a/src/audio/audio_encoder.zig +++ b/src/audio/audio_encoder.zig @@ -30,7 +30,6 @@ pub const EncodedAudioPacketNode = struct { pub const AudioEncoder = struct { const Self = @This(); - allocator: Allocator, audio_codec_ctx: [*c]ffmpeg.AVCodecContext, channels: u32, // A rolling buffer of raw audio waiting to be encoded. We only encode @@ -87,15 +86,14 @@ pub const AudioEncoder = struct { try checkErr(ret); return .{ - .allocator = allocator, .audio_codec_ctx = audio_codec_ctx, .channels = channels, .pending_samples = try .initCapacity(allocator, 0), }; } - pub fn deinit(self: *Self) void { - self.pending_samples.deinit(self.allocator); + pub fn deinit(self: *Self, allocator: Allocator) void { + self.pending_samples.deinit(allocator); ffmpeg.avcodec_free_context(&self.audio_codec_ctx); } @@ -104,6 +102,7 @@ pub const AudioEncoder = struct { /// the encoder. pub fn encode_chunk( self: *Self, + allocator: Allocator, start_sample: i64, pcm: []const f32, ) !?std.DoublyLinkedList { @@ -127,14 +126,14 @@ pub const AudioEncoder = struct { } } - try self.pending_samples.appendSlice(self.allocator, pcm); + try self.pending_samples.appendSlice(allocator, pcm); - return try self.encode_buffered_frames(false); + return try self.encode_buffered_frames(allocator, false); } /// Encode buffered samples if there are enough to fill the codec frame size. /// allow_partial - Set to true to ignore this check and encode anyway. - fn encode_buffered_frames(self: *Self, allow_partial: bool) !std.DoublyLinkedList { + fn encode_buffered_frames(self: *Self, allocator: Allocator, allow_partial: bool) !std.DoublyLinkedList { var audio_packets: std.DoublyLinkedList = .{}; errdefer deinit_packet_list(&audio_packets); @@ -162,7 +161,14 @@ pub const AudioEncoder = struct { // `sendFrame` reads from the front of `pending_samples`, so after it // returns we compact the remaining PCM to keep the rolling buffer // contiguous for the next capture chunk. - try self.send_frame(&audio_packets, frame, self.pending_start_sample.?, codec_samples_per_packet, submitted_samples); + try self.send_frame( + allocator, + &audio_packets, + frame, + self.pending_start_sample.?, + codec_samples_per_packet, + submitted_samples, + ); const consumed_samples = submitted_samples * self.channels; const remaining_samples = self.pending_samples.items.len - consumed_samples; @@ -171,7 +177,7 @@ pub const AudioEncoder = struct { self.pending_samples.items[0..remaining_samples], self.pending_samples.items[consumed_samples..], ); - try self.pending_samples.resize(self.allocator, remaining_samples); + try self.pending_samples.resize(allocator, remaining_samples); self.pending_start_sample.? += @intCast(submitted_samples); if (remaining_samples == 0) { @@ -183,21 +189,22 @@ pub const AudioEncoder = struct { return audio_packets; } - pub fn flush(self: *Self) !std.DoublyLinkedList { + pub fn flush(self: *Self, allocator: Allocator) !std.DoublyLinkedList { assert(!self.is_flushed); - var audio_packets = try self.encode_buffered_frames(true); + var audio_packets = try self.encode_buffered_frames(allocator, true); errdefer deinit_packet_list(&audio_packets); self.is_flushed = true; const ret = ffmpeg.avcodec_send_frame(self.audio_codec_ctx, null); try checkErr(ret); - try self.collect_ready_packets(&audio_packets); + try self.collect_ready_packets(allocator, &audio_packets); return audio_packets; } fn send_frame( self: *Self, + allocator: Allocator, audio_packets: *std.DoublyLinkedList, frame: [*c]ffmpeg.AVFrame, start_sample: i64, @@ -240,11 +247,12 @@ pub const AudioEncoder = struct { try checkErr(ret); // A single submitted frame can produce zero, one, or multiple packets // depending on encoder delay, so always drain after each send. - try self.collect_ready_packets(audio_packets); + try self.collect_ready_packets(allocator, audio_packets); } fn collect_ready_packets( self: *Self, + allocator: Allocator, audio_packets: *std.DoublyLinkedList, ) !void { var audio_pkt = ffmpeg.av_packet_alloc() orelse return error.FFmpegError; @@ -259,7 +267,7 @@ pub const AudioEncoder = struct { var owned_pkt = ffmpeg.av_packet_alloc() orelse return error.FFmpegError; errdefer ffmpeg.av_packet_free(&owned_pkt); ffmpeg.av_packet_move_ref(owned_pkt, audio_pkt); - const node = try EncodedAudioPacketNode.init(self.allocator, owned_pkt); + const node = try EncodedAudioPacketNode.init(allocator, owned_pkt); audio_packets.append(&node.node); } } @@ -276,13 +284,13 @@ test "encodeChunk rejects non-contiguous sample input" { const allocator = std.testing.allocator; var encoder = try AudioEncoder.init(allocator, 48_000, 2); - defer encoder.deinit(); + defer encoder.deinit(allocator); const pcm = [_]f32{ 0.0, 0.0, 0.0, 0.0 }; - var first_result = (try encoder.encode_chunk(0, &pcm)).?; + var first_result = (try encoder.encode_chunk(allocator, 0, &pcm)).?; defer deinit_packet_list(&first_result); try std.testing.expect(first_result.first == null); - try std.testing.expectError(error.NonContiguousAudioPts, encoder.encode_chunk(3, &pcm)); + try std.testing.expectError(error.NonContiguousAudioPts, encoder.encode_chunk(allocator, 3, &pcm)); } test "encodeChunk plus flush produces encoded audio packets" { @@ -293,7 +301,7 @@ test "encodeChunk plus flush produces encoded audio packets" { const sample_positions: usize = 2_048; var encoder = try AudioEncoder.init(allocator, sample_rate, channels); - defer encoder.deinit(); + defer encoder.deinit(allocator); const pcm = try allocator.alloc(f32, sample_positions * channels); defer allocator.free(pcm); @@ -308,12 +316,12 @@ test "encodeChunk plus flush produces encoded audio packets" { var all_packets: std.DoublyLinkedList = .{}; defer deinit_packet_list(&all_packets); - var encoded_packets = (try encoder.encode_chunk(start_sample, pcm)).?; + var encoded_packets = (try encoder.encode_chunk(allocator, start_sample, pcm)).?; while (encoded_packets.popFirst()) |node| { all_packets.append(node); } - var flushed_packets = try encoder.flush(); + var flushed_packets = try encoder.flush(allocator); while (flushed_packets.popFirst()) |node| { all_packets.append(node); } diff --git a/src/audio/audio_timeline.zig b/src/audio/audio_timeline.zig index d96e65e..7a0b4e1 100644 --- a/src/audio/audio_timeline.zig +++ b/src/audio/audio_timeline.zig @@ -85,7 +85,7 @@ pub const AudioTimeline = struct { channels: u32, ) !Self { var encoder = try AudioEncoder.init(allocator, sample_rate, channels); - errdefer encoder.deinit(); + errdefer encoder.deinit(allocator); return .{ .allocator = allocator, @@ -109,7 +109,7 @@ pub const AudioTimeline = struct { self.device_map.deinit(); deinitPacketList(&self.ready_packets); - self.encoder.deinit(); + self.encoder.deinit(self.allocator); } pub fn add_data(self: *Self, data: *AudioCaptureData) !void { @@ -164,7 +164,7 @@ pub const AudioTimeline = struct { try self.process_ready_timeline(true); - var flush_result = try self.encoder.flush(); + var flush_result = try self.encoder.flush(self.allocator); errdefer deinitPacketList(&flush_result); self.append_ready_packets(&flush_result); } @@ -227,7 +227,7 @@ pub const AudioTimeline = struct { ); defer mixed_pcm.deinit(self.allocator); - var packets = try self.encoder.encode_chunk(self.encoded_until_sample, mixed_pcm.items); + var packets = try self.encoder.encode_chunk(self.allocator, self.encoded_until_sample, mixed_pcm.items); if (packets) |*owned_packets| { errdefer deinitPacketList(owned_packets); self.append_ready_packets(owned_packets); diff --git a/src/capture/video/linux/pipewire/pipewire_video.zig b/src/capture/video/linux/pipewire/pipewire_video.zig index cdd5d88..46b2ee1 100644 --- a/src/capture/video/linux/pipewire/pipewire_video.zig +++ b/src/capture/video/linux/pipewire/pipewire_video.zig @@ -327,7 +327,7 @@ pub const PipewireVideo = struct { var params = try std.ArrayList(*pw.spa_pod).initCapacity(allocator, 0); errdefer params.deinit(allocator); for (formats) |format| { - var modifiers = try self.vulkan.query_format_modifiers(pipewire_util.spa_to_vk_format(format)); + var modifiers = try self.vulkan.query_format_modifiers(self.allocator, pipewire_util.spa_to_vk_format(format)); defer modifiers.deinit(self.allocator); if (modifiers.items.len == 0) { continue; diff --git a/src/file_picker/file_picker.zig b/src/file_picker/file_picker.zig index 040eaa0..e807de3 100644 --- a/src/file_picker/file_picker.zig +++ b/src/file_picker/file_picker.zig @@ -1,4 +1,5 @@ const std = @import("std"); +const Allocator = std.mem.Allocator; pub const FilePickerError = error{ PickerCancelled, @@ -12,18 +13,17 @@ pub const FilePicker = struct { vtable: *const VTable, const VTable = struct { - open_directory_picker: *const fn (*anyopaque, ?[]const u8) anyerror![]u8, - deinit: *const fn (*anyopaque) void, + open_directory_picker: *const fn (*anyopaque, Allocator, ?[]const u8) anyerror![]u8, }; /// Open a directory picker and return the selected directory path. /// The returned path is owned by the caller. /// initial_directory - Open in this directory if provided. - pub fn open_directory_picker(self: *Self, initial_directory: ?[]const u8) (FilePickerError || anyerror)![]u8 { - return self.vtable.open_directory_picker(self.ptr, initial_directory); - } - - pub fn deinit(self: *Self) void { - return self.vtable.deinit(self.ptr); + pub fn open_directory_picker( + self: *Self, + allocator: Allocator, + initial_directory: ?[]const u8, + ) (FilePickerError || anyerror)![]u8 { + return self.vtable.open_directory_picker(self.ptr, allocator, initial_directory); } }; diff --git a/src/file_picker/linux/xdg_desktop_portal_file_picker.zig b/src/file_picker/linux/xdg_desktop_portal_file_picker.zig index 81c4b72..59be583 100644 --- a/src/file_picker/linux/xdg_desktop_portal_file_picker.zig +++ b/src/file_picker/linux/xdg_desktop_portal_file_picker.zig @@ -1,4 +1,5 @@ const std = @import("std"); +const Allocator = std.mem.Allocator; const TokenManager = @import("../../common/linux/token_manager.zig"); const FilePicker = @import("../file_picker.zig").FilePicker; const FilePickerError = @import("../file_picker.zig").FilePickerError; @@ -35,10 +36,9 @@ const OpenDirectoryPickerContext = struct { pub const XdgDesktopPortalFilePicker = struct { const Self = @This(); - allocator: std.mem.Allocator, dbus: *gio.DBusConnection, - pub fn init(allocator: std.mem.Allocator) !*Self { + pub fn init() !Self { var err: ?*glib.Error = null; defer if (err) |e| e.free(); @@ -51,13 +51,9 @@ pub const XdgDesktopPortalFilePicker = struct { return error.Dbus; }; - const self = try allocator.create(Self); - errdefer allocator.destroy(self); - self.* = .{ - .allocator = allocator, + return .{ .dbus = dbus, }; - return self; } fn open_directory_picker_response( @@ -92,7 +88,7 @@ pub const XdgDesktopPortalFilePicker = struct { ); } - fn selected_directory_from_result(self: *Self, result: *glib.Variant) ![]u8 { + fn selected_directory_from_result(allocator: Allocator, result: *glib.Variant) ![]u8 { var result_dict: glib.VariantDict = undefined; glib.VariantDict.init(&result_dict, result); defer result_dict.clear(); @@ -114,24 +110,28 @@ pub const XdgDesktopPortalFilePicker = struct { const file_path = glib.filenameFromUri(first_uri, null, null) orelse return error.FileUriToPathFailed; defer glib.free(file_path); - return self.allocator.dupe(u8, std.mem.span(file_path)); + return allocator.dupe(u8, std.mem.span(file_path)); } - pub fn open_directory_picker(context: *anyopaque, initial_directory: ?[]const u8) ![]u8 { + pub fn open_directory_picker( + context: *anyopaque, + allocator: Allocator, + initial_directory: ?[]const u8, + ) ![]u8 { const self: *Self = @ptrCast(@alignCast(context)); - const request_token = try TokenManager.generate_token(self.allocator); - defer self.allocator.free(request_token); + const request_token = try TokenManager.generate_token(allocator); + defer allocator.free(request_token); const initial_directory_z = if (initial_directory) |directory| - try self.allocator.dupeZ(u8, directory) + try allocator.dupeZ(u8, directory) else null; - defer if (initial_directory_z) |directory| self.allocator.free(directory); + defer if (initial_directory_z) |directory| allocator.free(directory); const unique_name = std.mem.span(self.dbus.getUniqueName().?); - const request_path = try TokenManager.get_request_path(self.allocator, unique_name[1..], request_token); - defer self.allocator.free(request_path); + const request_path = try TokenManager.get_request_path(allocator, unique_name[1..], request_token); + defer allocator.free(request_path); const loop = glib.MainLoop.new(null, 0); defer loop.unref(); @@ -219,13 +219,11 @@ pub const XdgDesktopPortalFilePicker = struct { const result = ctx.response_data orelse return error.OpenDirectoryPickerFailed; defer result.unref(); - return self.selected_directory_from_result(result); + return selected_directory_from_result(allocator, result); } - pub fn deinit(context: *anyopaque) void { - const self: *Self = @ptrCast(@alignCast(context)); + pub fn deinit(self: *Self) void { self.dbus.unref(); - self.allocator.destroy(self); } pub fn file_picker(self: *Self) FilePicker { @@ -233,7 +231,6 @@ pub const XdgDesktopPortalFilePicker = struct { .ptr = self, .vtable = &.{ .open_directory_picker = open_directory_picker, - .deinit = deinit, }, }; } diff --git a/src/file_picker/windows/windows_file_picker.zig b/src/file_picker/windows/windows_file_picker.zig index 1feb821..112e0a9 100644 --- a/src/file_picker/windows/windows_file_picker.zig +++ b/src/file_picker/windows/windows_file_picker.zig @@ -1,39 +1,29 @@ const std = @import("std"); +const Allocator = std.mem.Allocator; const FilePicker = @import("../file_picker.zig").FilePicker; pub const WindowsFilePicker = struct { const Self = @This(); - allocator: std.mem.Allocator, - - pub fn init(allocator: std.mem.Allocator) !*Self { - const self = try allocator.create(Self); - errdefer allocator.destroy(self); - - self.* = .{ - .allocator = allocator, - }; - return self; + pub fn init() !Self { + return .{}; } - pub fn open_directory_picker(context: *anyopaque, initial_directory: ?[]const u8) ![]u8 { + // TODO: Take an allocator for the returned owned memory. + pub fn open_directory_picker(context: *anyopaque, _: Allocator, initial_directory: ?[]const u8) ![]u8 { const self: *Self = @ptrCast(@alignCast(context)); _ = self; _ = initial_directory; return error.NotImplemented; } - pub fn deinit(context: *anyopaque) void { - const self: *Self = @ptrCast(@alignCast(context)); - self.allocator.destroy(self); - } + pub fn deinit(_: *Self) void {} pub fn file_picker(self: *Self) FilePicker { return .{ .ptr = self, .vtable = &.{ .open_directory_picker = open_directory_picker, - .deinit = deinit, }, }; } diff --git a/src/main.zig b/src/main.zig index a24bbd9..38469a8 100644 --- a/src/main.zig +++ b/src/main.zig @@ -86,9 +86,9 @@ fn gui_app(allocator: std.mem.Allocator, parsed_args: ?args.Args) !void { var audio_capture_interface = _audio_capture.audio_capture(); defer audio_capture_interface.deinit(); - const platform_file_picker = try PlatformFilePicker.init(allocator); - var file_picker = platform_file_picker.file_picker(); - defer file_picker.deinit(); + var platform_file_picker = try PlatformFilePicker.init(); + defer platform_file_picker.deinit(); + var file_picker_interface = platform_file_picker.file_picker(); const platform_global_shortcuts = try PlatformGlobalShortcuts.init(allocator); var global_shortcuts = platform_global_shortcuts.global_shortcuts(); @@ -99,7 +99,7 @@ fn gui_app(allocator: std.mem.Allocator, parsed_args: ?args.Args) !void { allocator, vulkan, &video_capture_interface, - &file_picker, + &file_picker_interface, &audio_capture_interface, &global_shortcuts, ); diff --git a/src/state/user_settings_state.zig b/src/state/user_settings_state.zig index 989a160..bbdf3c5 100644 --- a/src/state/user_settings_state.zig +++ b/src/state/user_settings_state.zig @@ -142,7 +142,7 @@ pub const UserSettingsState = struct { } break :blk null; }; - const selected_directory = actor.file_picker.open_directory_picker(directory) catch |err| { + const selected_directory = actor.file_picker.open_directory_picker(self.allocator, directory) catch |err| { switch (err) { FilePickerError.PickerCancelled => { log.info("[select_output_directory] output directory selection cancelled", .{}); diff --git a/src/vulkan/vulkan.zig b/src/vulkan/vulkan.zig index 1f3cff5..89b840d 100644 --- a/src/vulkan/vulkan.zig +++ b/src/vulkan/vulkan.zig @@ -1,4 +1,5 @@ const std = @import("std"); +const Allocator = std.mem.Allocator; const assert = std.debug.assert; const imguiz = @import("imguiz").imguiz; @@ -384,7 +385,7 @@ pub const Vulkan = struct { } /// Caller owns the memory - must free - pub fn query_format_modifiers(self: *const Self, format: vk.Format) !std.ArrayList(u64) { + pub fn query_format_modifiers(self: *const Self, allocator: Allocator, format: vk.Format) !std.ArrayList(u64) { var modifiers_list = vk.DrmFormatModifierPropertiesListEXT{}; var props = vk.FormatProperties2{ .p_next = @ptrCast(&modifiers_list), @@ -393,17 +394,17 @@ pub const Vulkan = struct { self.instance.getPhysicalDeviceFormatProperties2KHR(self.physical_device, format, &props); - const format_mod_props = try self.allocator.alloc(vk.DrmFormatModifierPropertiesEXT, modifiers_list.drm_format_modifier_count); - defer self.allocator.free(format_mod_props); + const format_mod_props = try allocator.alloc(vk.DrmFormatModifierPropertiesEXT, modifiers_list.drm_format_modifier_count); + defer allocator.free(format_mod_props); modifiers_list.p_drm_format_modifier_properties = format_mod_props.ptr; self.instance.getPhysicalDeviceFormatProperties2KHR(self.physical_device, format, &props); - var modifiers = try std.ArrayList(u64).initCapacity(self.allocator, 0); + var modifiers = try std.ArrayList(u64).initCapacity(allocator, 0); for (format_mod_props) |modifier| { - try modifiers.append(self.allocator, modifier.drm_format_modifier); + try modifiers.append(allocator, modifier.drm_format_modifier); } return modifiers;