Gnurou / v4l2r

Rust bindings for V4L2
MIT License
21 stars 10 forks source link

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

Open FallingSnow opened 3 years ago

FallingSnow commented 3 years ago

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 commented 3 years ago

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?