All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: mchehab@redhat.com, linux-media@vger.kernel.org,
	vladimir.barinov@cogentembedded.com, linux-sh@vger.kernel.org,
	matsu@igel.co.jp
Subject: Re: [PATCH v3] adv7180: add more subdev video ops
Date: Tue, 21 May 2013 17:17:09 +0000	[thread overview]
Message-ID: <519BAC15.8000503@cogentembedded.com> (raw)
In-Reply-To: <201305211645.49257.hverkuil@xs4all.nl>

Hello.

On 05/21/2013 06:45 PM, Hans Verkuil wrote:

>>>> From: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
>>>> Add subdev video ops for ADV7180 video decoder.  This makes decoder usable on
>>>> the soc-camera drivers.
>>>> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>> ---
>>>> 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.

     We have used V4L2_FIELD_INTERLACED before and people reported
incorrect field ordering -- that's why we changed to what it is now. So this
might be an application failure?

WBR, Sergei


WARNING: multiple messages have this Message-ID (diff)
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: mchehab@redhat.com, linux-media@vger.kernel.org,
	vladimir.barinov@cogentembedded.com, linux-sh@vger.kernel.org,
	matsu@igel.co.jp
Subject: Re: [PATCH v3] adv7180: add more subdev video ops
Date: Tue, 21 May 2013 21:17:09 +0400	[thread overview]
Message-ID: <519BAC15.8000503@cogentembedded.com> (raw)
In-Reply-To: <201305211645.49257.hverkuil@xs4all.nl>

Hello.

On 05/21/2013 06:45 PM, Hans Verkuil wrote:

>>>> From: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
>>>> Add subdev video ops for ADV7180 video decoder.  This makes decoder usable on
>>>> the soc-camera drivers.
>>>> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>> ---
>>>> 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.

     We have used V4L2_FIELD_INTERLACED before and people reported
incorrect field ordering -- that's why we changed to what it is now. So this
might be an application failure?

WBR, Sergei


  reply	other threads:[~2013-05-21 17:17 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-13 19:21 [PATCH v3] adv7180: add more subdev video ops Sergei Shtylyov
2013-05-13 19:21 ` Sergei Shtylyov
2013-05-20 20:20 ` Sergei Shtylyov
2013-05-20 20:20   ` Sergei Shtylyov
2013-05-21  9:28 ` Hans Verkuil
2013-05-21  9:28   ` Hans Verkuil
2013-05-21  9:35 ` Hans Verkuil
2013-05-21  9:35   ` Hans Verkuil
2013-05-21 14:28   ` Sergei Shtylyov
2013-05-21 14:28     ` Sergei Shtylyov
2013-05-21 14:45     ` Hans Verkuil
2013-05-21 14:45       ` Hans Verkuil
2013-05-21 17:17       ` Sergei Shtylyov [this message]
2013-05-21 17:17         ` Sergei Shtylyov
2013-05-22  6:55         ` Hans Verkuil
2013-05-22  6:55           ` Hans Verkuil
2013-05-22 12:23           ` Sergei Shtylyov
2013-05-22 12:23             ` Sergei Shtylyov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=519BAC15.8000503@cogentembedded.com \
    --to=sergei.shtylyov@cogentembedded.com \
    --cc=hverkuil@xs4all.nl \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=matsu@igel.co.jp \
    --cc=mchehab@redhat.com \
    --cc=vladimir.barinov@cogentembedded.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.