On Tue 21 May 2013 16:28:08 Sergei Shtylyov wrote:
> Hello.
>
> On 21-05-2013 13:35, Hans Verkuil wrote:
>
> >> From: Vladimir Barinov <vladimir.barinov@xxxxxxxxxxxxxxxxxx>
>
> >> Add subdev video ops for ADV7180 video decoder. This makes decoder usable on
> >> the soc-camera drivers.
>
> >> Signed-off-by: Vladimir Barinov <vladimir.barinov@xxxxxxxxxxxxxxxxxx>
> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
>
> >> ---
> >> This patch is against the 'media_tree.git' repo.
>
> >> Changes from version 2:
> >> - set the field format depending on video standard in try_mbus_fmt() method;
> >> - removed querystd() method calls from try_mbus_fmt() and cropcap() methods;
> >> - removed g_crop() method.
>
> >> drivers/media/i2c/adv7180.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 86 insertions(+)
> >>
> >> Index: media_tree/drivers/media/i2c/adv7180.c
> >> ===================================================================
> >> --- media_tree.orig/drivers/media/i2c/adv7180.c
> >> +++ media_tree/drivers/media/i2c/adv7180.c
>
>
> >> +
> >> +static int adv7180_try_mbus_fmt(struct v4l2_subdev *sd,
> >> + struct v4l2_mbus_framefmt *fmt)
> >> +{
> >> + struct adv7180_state *state = to_state(sd);
> >> +
> >> + fmt->code = V4L2_MBUS_FMT_YUYV8_2X8;
> >> + fmt->colorspace = V4L2_COLORSPACE_SMPTE170M;
> >> + fmt->field = state->curr_norm & V4L2_STD_525_60 ?
> >> + V4L2_FIELD_INTERLACED_BT : V4L2_FIELD_INTERLACED_TB;
>
> > Just noticed this: use V4L2_FIELD_INTERLACED as that does the right thing.
> > No need to split in _BT and _TB.
>
> Hm, testers have reported that _BT vs _TB do make a difference. I'll
> try to look into how V4L2 handles interlacing for different standards.
When using V4L2_FIELD_INTERLACED the BT vs TB is implicit (i.e. the application
is supposed to know that the order will be different depending on the standard).
Explicitly using BT/TB is only useful if you want to override the default, e.g.
if a video was encoded using the wrong temporal order.
> >> + fmt->width = 720;
> >> + fmt->height = state->curr_norm & V4L2_STD_525_60 ? 480 : 576;
> >> +
> >> + return 0;
> >> +}
>
> > Actually, all this code can be simplified substantially: the try/g/s_mbus_fmt
> > functions are really all identical since the data returned is only dependent
> > on the current standard. So this means you can use just a single function for
> > all three ops, and you can do away with adding struct v4l2_mbus_framefmt to
> > adv7180_state.
>
> Hm, I wonder how "get" and "set" methods can be identical... I'll
> look into this some more.
Normally they aren't, but in this case there is only one possible format, so
any values you attempt to set are ignored completely.
Regards,
Hans
--
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
