Hello Shoabo,
On Sat, Mar 02, 2019 at 03:29:25PM -0700, Shaobo He wrote:
> Hi Laurent,
>
> Thank you very much for your reply. This is what I thought, too. It seems that
> there's an implicit order of execution that is not clearly implied in the code,
> meaning `uvc_parse_streaming` is called before `uvc_v4l2_try_format`.
It is implied by the logic of the driver that calls
uvc_parse_streaming() at probe time before registering video nodes, and
with uvc_v4l2_try_format() only being called from userspace through the
video nodes.
> That being said, I was wondering maybe a better practice to write the loop in
> `uvc_v4l2_try_format` would be like the following,
>
> ```
> format=NULL;
> ...
> for (i = 0; i < stream->nformats; ++i) {
> format = &stream->format[i];
> if (format->fcc == fmt->fmt.pix.pixelformat)
> break;
> }
> // dereferencing format
> ```
> to
> ```
> // just declaration
> format;
> i=0;
> do {
> format = &stream->format[i];
> if (format->fcc == fmt->fmt.pix.pixelformat)
> break;
> ++i;
> } while (i<stream->nformats)
> // dereferencing format
> ```
> I mean you can save one initialization, provided compiler does it and one branch.
I like for loops better in general, they convey the meaning in a cleaner
way. The compiler should be able to do its job here and optimize the
code correctly, I don't think a change is worth it, especially as we're
not dealing with a hot path.
> On 2019/3/2 14:43, Laurent Pinchart wrote:
> > On Sat, Mar 02, 2019 at 01:22:49PM -0700, Shaobo He wrote:
> >> Hello everyone,
> >>
> >> This is Shaobo from Utah again. I've been bugging the mailing list with my
> >> patches. I have a quick question about a function in
> >> `drivers/media/usb/uvc/uvc_v4l2.c`. In `uvc_v4l2_try_format`, can
> >> `stream->nformats` be 0? I saw that in other files, this field could be zero
> >> which is considered as error cases. I was wondering if it's true for this
> >> function, too.
> >
> > The uvc_parse_streaming() function should answer this question :-)
--
Regards,
Laurent Pinchart
