Linux Media Controller development
 help / color / mirror / Atom feed
From: "Niklas Söderlund" <niklas.soderlund@ragnatech.se>
To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	slongerbeam@gmail.com, lars@metafoo.de, mchehab@kernel.org,
	hans.verkuil@cisco.com
Subject: Re: [PATCH 1/6] media: rcar-vin: allow field to be changed
Date: Mon, 1 Aug 2016 18:52:07 +0200	[thread overview]
Message-ID: <20160801165207.GD3672@bigcity.dyn.berto.se> (raw)
In-Reply-To: <1f735458-9ff3-f3fc-e349-20ac0b57cde1@cogentembedded.com>

Hi Sergei,

Thanks for testing!

On 2016-07-30 00:04:33 +0300, Sergei Shtylyov wrote:
> On 07/29/2016 08:40 PM, Niklas Söderlund wrote:
> 
> > The driver forced whatever field was set by the source subdevice to be
> > used. This patch allows the user to change from the default field.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
>    I didn't apply this patch at first (thinking it was unnecessary), and the
> capture worked fine. The field order appeared swapped again after I did
> import this patch as well. :-(

I had a look at the test tool you told me you use 
(https://linuxtv.org/downloads/v4l-dvb-apis/capture-example.html) and 
the reason the field order is swapped is a combination of that tool and 
how the rcar-vin driver interprets V4L2_FIELD_INTERLACED.

1. The tool you use asks for V4L2_FIELD_INTERLACED if the -f switch is 
   used. You told me #v4l that you do use that switch, but have modified 
   the tool to use a different pixelformat than used in the link above, 
   correct?

2. The rcar-vin driver interprets V4L2_FIELD_INTERLACED as 
   V4L2_FIELD_INTERLACED_TB. If this is correct or not I do not know, 
   the old soc-camera version of the driver do it this way so I have 
   kept that logic.

This is the reason why the field order is wrong when you apply this 
patch. Without it the field order would be locked to whatever the 
subdevice reports, V4L2_FIELD_ALTERNATE in this case.

I don't know if it's correct to treat V4L2_FIELD_INTERLACED as 
V4L2_FIELD_INTERLACED_TB or if I should try and use G_STD if 
V4L2_FIELD_INTERLACED is requested and change the field to _TB or _BT 
according to the result of that. I feel this will only push the problem 
further down. What if G_STD is not implemented by the subdevice? Then a 
default fallback interpretation to ether _TB or _BT would still be 
needed. I'm open to suggestion on how to handle this case.

There is also a feature missing in this patch. The field order was set 
to V4L2_FIELD_NONE if the requested format asks for V4L2_FIELD_ANY.  
This have no effect for how you test but i did run into it while trying 
to figure this out. I will send out a v2 which solves this by retaining 
the current field mode if V4L2_FIELD_ANY is asked for.

> 
> MBR, Sergei
> 

-- 
Regards,
Niklas Söderlund

  reply	other threads:[~2016-08-01 16:52 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-29 17:40 [PATCH 0/6] Fix adv7180 and rcar-vin field handling Niklas Söderlund
2016-07-29 17:40 ` [PATCH 1/6] media: rcar-vin: allow field to be changed Niklas Söderlund
2016-07-29 21:04   ` Sergei Shtylyov
2016-08-01 16:52     ` Niklas Söderlund [this message]
2016-08-01 17:55       ` Hans Verkuil
2016-07-29 17:40 ` [PATCH 2/6] media: rcar-vin: fix bug in scaling Niklas Söderlund
2016-07-29 17:40 ` [PATCH 3/6] media: rcar-vin: fix height for TOP and BOTTOM fields Niklas Söderlund
2016-07-29 17:40 ` [PATCH 4/6] media: rcar-vin: add support for V4L2_FIELD_ALTERNATE Niklas Söderlund
2016-07-30 21:55   ` Sergei Shtylyov
2016-08-01 16:53     ` Niklas Söderlund
2016-08-02  9:41   ` Hans Verkuil
2016-08-02 10:32     ` Niklas Söderlund
2016-08-02 10:39       ` Hans Verkuil
2016-08-02 11:02         ` Niklas Söderlund
2016-08-02 11:21           ` Hans Verkuil
2016-07-29 17:40 ` [PATCH 5/6] media: adv7180: fill in mbus format in set_fmt Niklas Söderlund
2016-07-29 17:40 ` [PATCH 6/6] media: adv7180: fix field type Niklas Söderlund
2016-07-29 19:10   ` Sergei Shtylyov
2016-07-29 19:32     ` Steve Longerbeam
2016-07-29 20:16       ` Niklas Söderlund
2016-08-02  9:43 ` [PATCH 0/6] Fix adv7180 and rcar-vin field handling Hans Verkuil

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=20160801165207.GD3672@bigcity.dyn.berto.se \
    --to=niklas.soderlund@ragnatech.se \
    --cc=hans.verkuil@cisco.com \
    --cc=lars@metafoo.de \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sergei.shtylyov@cogentembedded.com \
    --cc=slongerbeam@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox