Skip to content

Comments

feat: add core types and update documentation#7

Closed
polw1 wants to merge 3 commits intosilvermine:masterfrom
polw1:types
Closed

feat: add core types and update documentation#7
polw1 wants to merge 3 commits intosilvermine:masterfrom
polw1:types

Conversation

@polw1
Copy link
Contributor

@polw1 polw1 commented Sep 11, 2025

This PR adds documentation for the media-parser crate, including examples of how to use the public API.
It also adds the Rust code definitions for the core types and their structs.

Key features covered in the documentation:

  • Retrieving metadata (title, artist, album, duration, etc.)
  • Listing tracks with type, codec, and language
  • Extracting subtitles by track ID or language
  • Capturing single or multiple video frames


/// Trait for reading media streams from various sources
#[async_trait]
pub trait StreamReader: Send + Sync {
Copy link
Contributor

Choose a reason for hiding this comment

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

read and seek should probably be modified to not take &mut self as this complicates ownership.

We may be able to use interior mutability Arc<Mutex<>> to hide the mutable state from the public API.

Copy link
Contributor Author

@polw1 polw1 Sep 25, 2025

Choose a reason for hiding this comment

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

I wonder if pushing mutability inside the reader could hurt performance.

For example:

reader.seek(SeekFrom::Start(100)).await?;
reader.read(&mut buf).await?; // lock during async I/O

To avoid a race between seek and read, we’d need to hold the same lock across both operations—which means holding a lock while awaiting I/O.

Considering how we possibly will use the reader, i think it makes sense to use a read_at/size approach and avoid all these problems. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, @polw1. Agree with your suggestion to update the StreamReader trait with a read_at function to eliminate the need for interior mutability. This not only resolves the mutability issue but also enables parallel I/O reads without locks.

It also aligns with patterns used in other crates to support stateless reads with parallel access.

Copy link
Contributor

@velocitysystems velocitysystems left a comment

Choose a reason for hiding this comment

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

Handing over to @yokuze for review.

polw1 and others added 2 commits November 6, 2025 07:18
Added Silvermine’s Rust standards and supporting configuration files:
 * rust-toolchain.toml for version pinning
 * rustfmt.toml with formatting rules
 * .cargo/config.toml with lint/fix aliases
 * Updated README with linting instructions

These additions ensure consistency across Rust projects and align with
organization-wide practices.
feat: update StreamReader trait to use read_at approach
@polw1 polw1 closed this Nov 6, 2025
@polw1 polw1 deleted the types branch November 20, 2025 19:48
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.

3 participants