Em 22-09-2011 12:13, Marek Szyprowski escreveu:
> Hello Mauro,
>
> I've collected pending selection API patches together with pending
> videobuf2 and Samsung driver fixes to a single git branch. Please pull
> them to your media tree.
>
> Marek Szyprowski (1):
> staging: dt3155v4l: fix build break
I've applied this one previously, from the patch you sent me.
> Tomasz Stanislawski (6):
> v4l: add support for selection api
> v4l: add documentation for selection API
I need more time to review those two patches. I'll probably do it at the next week.
I generally start analyzing API changes based on the DocBook, so, let me point a few
things I've noticed on a quick read, at the vidioc-g-selection.html DocBook-generated page:
1) "The coordinates are expressed in driver-dependant units"
Why? coordinates should be expressed in pixels, as otherwise there's no way to
use this API on a hardware-independent way.
2)
0 - driver is free to adjust size, it is recommended to choose the crop/compose rectangle as close as possible to the original one
SEL_SIZE_GE - driver is not allowed to shrink the rectangle. The original rectangle must lay inside the adjusted one
SEL_SIZE_LE - drive is not allowed to grow the rectangle. The adjusted rectangle must lay inside the original one
SEL_SIZE_GE | SEL_SIZE_LE - choose size exactly the same as in desired rectangle.
The macro names above don't match the definition, as they aren't prefixed by V4L2_.
3) There was no hyperlink for the struct v4l2_selection, as on other API definitions.
4) the language doesn't seem too consistent with the way other ioctl's are defined. For example,
you're using struct::field for a field at the struct. Other parts of the API just say "field foo of struct bar".
5) There's not a single mention at the git commit or at the DocBook about why the old crop API
is being deprecated. You need to convince me about such need (ok, I followed a few discussions in
the past, but, my brain patch buffer is shorter than the 7000 patchwork patches I reviewed just on
this week). Besides that: do we really need to obsolete the crop API for TV cards? If so, why? If not,
you need to explain why a developer should opt between one ioctl set of the other.
6) You should add a note about it at hist-v4l2.html page, stating what happened, and why a new crop
ioctl set is needed.
7) You didn't update the Experimental API Elements or the Obsolete API Elements at the hist-v4l2.html
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
