On Sat, 7 Aug 2010, Hans Verkuil wrote:
> On Friday 06 August 2010 22:21:36 Guennadi Liakhovetski wrote:
> > On Fri, 6 Aug 2010, Laurent Pinchart wrote:
> >
> > > Hi Guennadi,
> > >
> > > On Thursday 05 August 2010 20:03:46 Guennadi Liakhovetski wrote:
> > > > Many video drivers implement a discrete set of frame formats and thus face
> > > > a task of finding the best match for a user-requested format. Implementing
> > > > this in a generic function has also an advantage, that different drivers
> > > > with similar supported format sets will select the same format for the
> > > > user, which improves consistency across drivers.
> > > >
> > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> > > > ---
> > > >
> > > > I'm currently away from my hardware, so, this is only compile tested and
> > > > run-time tested with a test application. In any case, reviews and
> > > > suggestions welcome.
> > > >
> > > > drivers/media/video/v4l2-common.c | 26 ++++++++++++++++++++++++++
> > > > include/linux/videodev2.h | 10 ++++++++++
> > > > 2 files changed, 36 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/media/video/v4l2-common.c
> > > > b/drivers/media/video/v4l2-common.c index 4e53b0b..90727e6 100644
> > > > --- a/drivers/media/video/v4l2-common.c
> > > > +++ b/drivers/media/video/v4l2-common.c
> > > > @@ -1144,3 +1144,29 @@ int v4l_fill_dv_preset_info(u32 preset, struct
> > > > v4l2_dv_enum_preset *info) return 0;
> > > > }
> > > > EXPORT_SYMBOL_GPL(v4l_fill_dv_preset_info);
> > > > +
> > > > +struct v4l2_frmsize_discrete *v4l2_find_nearest_format(struct
> > > > v4l2_discrete_probe *probe, + s32 width, s32 height)
> > > > +{
> > > > + int i;
> > > > + u32 error, min_error = ~0;
> > > > + struct v4l2_frmsize_discrete *size, *best = NULL;
> > > > +
> > > > + if (!probe)
> > > > + return best;
> > > > +
> > > > + for (i = 0, size = probe->sizes; i < probe->num_sizes; i++, size++) {
> > > > + if (probe->probe && !probe->probe(probe))
> > >
> > > What's this call for ?
> >
> > Well, atm, I don't think I have a specific case right now, but I think, it
> > can well be the case, that not all frame sizes are always usable in the
> > driver, depending on other circumstances. E.g., depending on the pixel /
> > fourcc code. So, in this case the driver just provides a probe method to
> > filter out inapplicable sizes.
>
> Never add code just because you think it might be needed in the future. Especially
> not for kernel internal code that you can change anyway if needed.
Yes and no. If it's pretty obvious, that a certain extension is going to
be needed in the future, then it can be better to prepare for it, because
otherwise developers can start implementing (ugly custom) local
workarounds and extensions, before someone comes up with the idea to
extend the API, and then you have to convert all those special solutions.
Whereas if you prepare such an API, even if it is not perfect, people are
more likely to modify and fix it, than to extend an "established API."
But if in this case noone says - yes, my driver will need to be able to
filter certain sizes out, we can as well drop this callback.
> In this case you just want to give a simple const array with width and height
> pairs ending at 0, 0 or pass in the length of the array, whatever you prefer,
> and return the array index for the closest match.
>
> Keep it simple.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
