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

v4l2r::device::queue::FormatBuilder does not follow rust builder convention #8

Open
FallingSnow opened this issue Apr 23, 2021 · 1 comment

Comments

@FallingSnow
Copy link
Contributor

FallingSnow commented Apr 23, 2021

I believe v4l2r::device::queue::FormatBuilder.apply() should be changed to .build() and should not set the format. Instead you should use queue.set_format().

I thought that apply was the equivalent was of build because set != apply in my mind. I think of apply and I think: ok, I'm done, give me the result (aka build). For example most people hit apply before hitting ok in a settings dialog. I didn't think that it would actually set the format.

Another option is to give a .build() and a .set() option.

That's my opinion, feel free to close if you disagree.

@Gnurou
Copy link
Owner

Gnurou commented May 2, 2021

Basically I agree with your suggestion. The reason for the current design is so the decoder can just pass a FormatBuilder to the format change callback and have user code negotiate the format directly without getting full access to the queue. FormatBuilder is basically a proxy to the queue that only provides access to set_format() and try_format(). We don't want the format change callback to e.g. request buffers as the decoder will do that itself once the format is established.

So rather than a change of semantics, maybe we need a change of name here?

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

No branches or pull requests

2 participants