Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Channel API #2451

Merged
merged 9 commits into from
Aug 31, 2020
Merged

Channel API #2451

merged 9 commits into from
Aug 31, 2020

Conversation

lu-zero
Copy link
Collaborator

@lu-zero lu-zero commented Jul 15, 2020

This set provides a channel-based API that should make much simpler the normal usage.

TODO:

  • Basic encoding API
    • Works
    • Produces the same output for the same input
  • Multipass encoding API
    • Single pass
    • Second pass
      • Produces the same output for the same input
    • Multipass

Open questions:

  • Fit a last-error inside the ReceiverPacket abstraction and a mean to retrieve it
  • For multipass the api can be a single function and use the Config rc information to decide if it has to be multipass/firstpass/secondpass/normal

Solves: #689

(apparently github got some consistency issue with #2118 so I ended up opening this)

@lu-zero lu-zero force-pushed the channel-api branch 3 times, most recently from 6f3abec to dbe4217 Compare July 18, 2020 06:17
@coveralls
Copy link
Collaborator

coveralls commented Jul 18, 2020

Coverage Status

Coverage decreased (-0.001%) to 80.789% when pulling a0b31d4 on rust-av:channel-api into 8e3e361 on xiph:master.

@lu-zero lu-zero force-pushed the channel-api branch 3 times, most recently from 54975b0 to 98fd0ea Compare July 24, 2020 10:38
@lu-zero lu-zero requested a review from tdaede July 24, 2020 16:13
@shssoichiro shssoichiro self-requested a review July 28, 2020 13:34
Copy link
Contributor

@Urhengulas Urhengulas left a comment

Choose a reason for hiding this comment

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

I have a couple of questions and suggestions 😄

if self.limit < self.count {
Err(TrySendError::Full(data))
} else {
let r = self.sender.try_send(data);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use match?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it makes the code more verbose.

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually think it makes it easier to read. But maybe just a preference.

});

// Receive Packets
let d = s.spawn(move |_| -> Result<(), CliError> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe not use d as variable name?


info!(
"Using y4m decoder: {}x{}p @ {}/{} fps, {}, {}-bit",
video_info.width,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe also move these attributes out to make it less verbose?

let width = video_info.width;
// ...

@lu-zero
Copy link
Collaborator Author

lu-zero commented Aug 31, 2020

Now with the commits collapsed without the interims :)

@lu-zero lu-zero merged commit 05747ee into xiph:master Aug 31, 2020
@lu-zero lu-zero deleted the channel-api branch August 31, 2020 15:39
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.

5 participants