Skip to content

Initial support for adding new rows to a page - #220

Merged
Holzhaus merged 6 commits into
Holzhaus:mainfrom
RobinMcCorkell:offset3
Mar 19, 2026
Merged

Initial support for adding new rows to a page#220
Holzhaus merged 6 commits into
Holzhaus:mainfrom
RobinMcCorkell:offset3

Conversation

@RobinMcCorkell

Copy link
Copy Markdown
Contributor

No description provided.

@acrilique

Copy link
Copy Markdown
Collaborator

have been taking a look and playing with this (combined with the database branch). with a few fixes I have managed to generate an export that does get read correctly by my cdj-350 players by reading an entire database, creating a new empty database with the same table layout, inserting all the rows in the same order and serializing. very cool

@acrilique

Copy link
Copy Markdown
Collaborator

I got a simple mp3 exporter working. The database and audio files are read by the pioneer cdj-350 correctly. Take a look if you have time https://github.com/acrilique/rekordcrate/tree/pdb-from-scratch

@RobinMcCorkell

Copy link
Copy Markdown
Contributor Author

I got a simple mp3 exporter working. The database and audio files are read by the pioneer cdj-350 correctly. Take a look if you have time https://github.com/acrilique/rekordcrate/tree/pdb-from-scratch

Nice! I think we should get some stuff merged so we can build on top of each others' work more easily.

@acrilique

Copy link
Copy Markdown
Collaborator

This PR is highly related to #173

@Swiftb0y Swiftb0y left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple thoughts

Comment thread src/pdb/mod.rs
Comment on lines +662 to +667
trait PageHeapObject {
type Args<'a>;

/// Required page heap space in bytes to store the object.
fn heap_bytes_required(&self, args: Self::Args<'_>) -> u16;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats your opinion on writing a custom derive for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be useful to reduce boilerplate for sure but adds an extra crate that we would need to version and track. I think that it's probably not worth the overhead for now.

Comment thread src/pdb/offset_array.rs Outdated
Comment thread src/pdb/mod.rs
Comment thread src/pdb/test_modification.rs
Comment thread src/pdb/mod.rs Outdated

@Swiftb0y Swiftb0y left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Lgtm. @acrilique or @Holzhaus, do you have any opinion on this?

@acrilique acrilique left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also looks good to me. Have been playing w this one (as I said in previous comments) and thus am excited to get it merged.

@Holzhaus Holzhaus left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you very much. Excited for this one!

@Holzhaus
Holzhaus merged commit e60027a into Holzhaus:main Mar 19, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants