Applications: Add SampleEditor audio editing application by macsplit · Pull Request #26509 · SerenityOS/serenity

macsplit · 2025-12-27T22:47:06Z

Part 1 of 2. This PR contains the core logic for SampleEditor, refactored to remove redundant abstractions as suggested in #26402.

Changes from original PR:

Removed clipboard operations (copy/cut/paste) - will be added in Part 2
Integrated SampleNullBlock into SampleBlock as a concrete implementation using a static factory method (SampleBlock::create_null())
Removed SampleFile abstraction - SampleSourceFile now inherits directly from RefCounted since it was the only implementation

The application supports:

Opening and playing WAV/FLAC files
Waveform visualization with zoom controls
Selection management (select all, clear selection)
Saving audio files

Part 2 will add clipboard operations and editing capabilities (cut/copy/paste with reference-based architecture).

LucasChollet · 2026-01-07T09:39:44Z

FYI, CI is red

BuggieBot · 2026-01-09T17:35:18Z

Hello!

One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

macsplit · 2026-01-11T14:57:19Z

Okay, finally it's passing.

SampleEditor is a graphical audio sample editor supporting WAV and FLAC file formats. It provides essential audio editing functionality including playback, waveform visualization, and selection management. Key features: - WAV and FLAC format support for loading and saving - Real-time audio playback with configurable start positions - Reference-based architecture for memory-efficient editing - Zoom controls for waveform inspection - Keyboard shortcuts for all common operations The application uses a block-based design where audio content is represented as file references. This will allow efficient editing of large files without loading everything into RAM. This is Part 1 of a two-part implementation. Part 2 will add clipboard operations (copy/cut/paste).

LucasChollet

I started to read the code and left a few comments. I also tested the app at the same time, and I was a bit disappointed...
It seems that you're parsing the audio file on every frame of the render loop, which makes the application unusable with the two test files that I got. Can you fix that?

Also, the class hierarchy is quite difficult to understand, what is a SampleBlock vs SampleFileBlock vs SampleBuffer vs SampleSourceFile. They all have common methods while not having a clear goal. Let's talk about it on Discord.

It would also be nice to find a way to split SampleWidget a bit.

LucasChollet · 2026-02-09T14:35:31Z

Userland/Applications/SampleEditor/CMakeLists.txt

@@ -0,0 +1,21 @@
+serenity_component(
+    SampleEditor
+    REQUIRED


I know the app is great and all, but let's use RECOMMENDED instead 😄

LucasChollet · 2026-02-09T14:59:32Z

Userland/Applications/SampleEditor/MainWidget.cpp

+    auto source_file = TRY(try_make_ref_counted<SampleSourceFile>(path));
+    size_t length = source_file->length();
+    auto file_block = TRY(try_make_ref_counted<SampleFileBlock>(
+        source_file, (size_t)0, (size_t)length - 1));


It seems that you're always creating a SampleSourceFile to pass it to a SampleFileBlock.
Please make SampleFileBlock's constructor take a path and create the SourceFile itself.

The code will be a bit neater but will also make it easier to understand the class hierarchy.

LucasChollet · 2026-02-09T15:03:41Z

Userland/Applications/SampleEditor/MainWidget.cpp

+    m_new_action = GUI::Action::create(
+        "&New", { Mod_Ctrl, Key_N },
+        TRY(Gfx::Bitmap::load_from_file("/res/icons/16x16/new.png"sv)),


No required action: Seems like we could use a make_new_action factory in the project, multiple place would benefit.

LucasChollet · 2026-02-09T15:44:28Z

Userland/Applications/SampleEditor/SampleFormatStruct.h

+#include <AK/String.h>
+#include <AK/Types.h>
+
+typedef struct SampleFormat {


Again, this is not C, let's use a struct with no typedef

LucasChollet · 2026-02-09T15:45:44Z

Userland/Applications/SampleEditor/SampleSourceFile.h

+    RenderStruct rendered_sample_within_buffer(FixedArray<Audio::Sample>& buffer, size_t at);
+    String m_filename;
+    FixedArray<Audio::Sample> m_buffer;
+    StringView m_format_name;


Seems unused.

macsplit · 2026-02-09T18:29:59Z

Okay will look through this feedback

github-actions · 2026-03-03T02:13:38Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

kleinesfilmroellchen

Haven’t looked at the GUI code too closely, but the audio code obviously. There’s a lot of things to fix here still.

kleinesfilmroellchen · 2026-03-04T14:42:42Z

Userland/Applications/SampleEditor/main.cpp

+    TRY(Core::System::unveil("/etc", "r"));
+    TRY(Core::System::unveil("/res", "r"));
+    TRY(Core::System::unveil("/home", "rwc"));
+    TRY(Core::System::unveil("/home/anon", "rwc"));


Huh, I thought we didn’t have those anymore. If you want to properly unveil with arbitrary user file access, I think the pattern was FileSystemAccessClient (and remove /home from the unveil list).

(Doesn’t FSAC have a mode where it gives you an open fd without you having to open the file yourself?)

kleinesfilmroellchen · 2026-03-04T14:42:50Z

Userland/Applications/SampleEditor/main.cpp

+    window->set_main_widget(main_widget);
+    TRY(main_widget->initialize_menu_and_toolbar(window));
+
+    TRY(Core::System::unveil("/", "r"));


This seems unnecessary.

kleinesfilmroellchen · 2026-03-04T14:44:05Z

Userland/Applications/SampleEditor/main.cpp

+    TRY(Core::System::unveil(nullptr, nullptr));
+
+    if (arguments.argc > 1) {
+        TRY(main_widget->open(arguments.strings[1]));


Note: the application should still start even if the file provided is inaccessible or invalid.

(Sidenote: you can also specifically unveil this path as rw, so FSAC doesn’t need to be invoked on program startup)

kleinesfilmroellchen · 2026-03-04T14:44:20Z

Userland/Applications/SampleEditor/main.cpp

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2025, Lee Hanken


Copyright date is wrong here and many other files

kleinesfilmroellchen · 2026-03-04T14:48:15Z

Userland/Applications/SampleEditor/RenderStruct.h

Note for this and the following sample-related code: If it contains no GUI code, it can go into LibDSP. I will need that code for Piano too :)

kleinesfilmroellchen · 2026-03-04T15:06:09Z

Userland/Applications/SampleEditor/SampleSourceFile.cpp

+        double val = buffer[at].left;
+        double square = val * val;
+        double mod = AK::abs(val);
+


Please improve the variable names val and mod.

kleinesfilmroellchen · 2026-03-04T15:07:01Z

Userland/Applications/SampleEditor/SampleSourceFile.cpp

+    size_t end = max(size, at + window);
+
+    size_t samples = max(window, 25);
+    size_t increment = window / samples;


Make sure that this is not zero, otherwise the loop below is infinite.

kleinesfilmroellchen · 2026-03-04T15:07:21Z

Userland/Applications/SampleEditor/SampleSourceFile.cpp

+
+    RenderStruct value = { 0, 0, 0, 0 };
+
+    if (count_plus) {


use > 0, this is not C code golf

kleinesfilmroellchen · 2026-03-04T15:08:14Z

Userland/Applications/SampleEditor/SampleSourceFile.cpp

+
+void SampleSourceFile::begin_loading_samples() { m_stream_position = 0; }
+
+FixedArray<Audio::Sample> SampleSourceFile::load_more_samples()


Why are you not calling this function above in SampleFileBlock?

kleinesfilmroellchen · 2026-03-04T15:12:32Z

Userland/Applications/SampleEditor/SampleWidget.cpp

+        }
+
+        m_current_audio_buffer.swap(buffer);
+        MUST(m_audio_connection->async_enqueue(m_current_audio_buffer));


The enqueue API is intended for noninteractive players, which SampleEditor isn’t. You should instead be reaching directly into the underlying buffer API, which gives you better latency on user actions. I believe that Piano already does so, so you should check the code there (instead of the other multimedia apps).

github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Dec 27, 2025

macsplit force-pushed the sample-editor-basic branch 3 times, most recently from 135235f to 4d5a791 Compare January 9, 2026 17:28

macsplit force-pushed the sample-editor-basic branch 4 times, most recently from 929c23f to c19dba6 Compare January 11, 2026 00:53

macsplit force-pushed the sample-editor-basic branch from c19dba6 to d161831 Compare February 1, 2026 00:54

LucasChollet reviewed Feb 9, 2026

View reviewed changes

github-actions bot added the stale label Mar 3, 2026

kleinesfilmroellchen suggested changes Mar 4, 2026

View reviewed changes

github-actions bot removed the stale label Mar 5, 2026


		void SampleSourceFile::begin_loading_samples() { m_stream_position = 0; }

		FixedArray<Audio::Sample> SampleSourceFile::load_more_samples()

Conversation

macsplit commented Dec 27, 2025

Changes from original PR:

Uh oh!

LucasChollet commented Jan 7, 2026

Uh oh!

BuggieBot commented Jan 9, 2026

Uh oh!

macsplit commented Jan 11, 2026

Uh oh!

LucasChollet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

macsplit commented Feb 9, 2026

Uh oh!

github-actions bot commented Mar 3, 2026

Uh oh!

kleinesfilmroellchen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants