Message ID | 1460794340-490-1-git-send-email-ivo.g.dimitrov.75@gmail.com (mailing list archive) |
---|---|
State | New |
Delegated to: | Sakari Ailus |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1arLLq-0003Nn-JO; Sat, 16 Apr 2016 08:13:10 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.76/mailfrontend-7) with esmtp id 1arLLl-00076b-2Q; Sat, 16 Apr 2016 10:13:07 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751316AbcDPIMl (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Sat, 16 Apr 2016 04:12:41 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:34933 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751100AbcDPIMi (ORCPT <rfc822;linux-media@vger.kernel.org>); Sat, 16 Apr 2016 04:12:38 -0400 Received: by mail-wm0-f66.google.com with SMTP id a140so11329240wma.2; Sat, 16 Apr 2016 01:12:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=IQlbAsWIdgnRzHlP1pG9xWwKdhyZdUkCDTuuG+0hh0M=; b=jRlVArDE1qwUeaac/Dg5/uypz3UxOVKtf4RdetPAdKQowUY76qe5GWMQfrJmY6czk7 nxm6yOWIZ5Vs3uyaj/5vbHBe+EWIYLCRDUUyRqeHdwlOqSZf9vMrwJsp6VCBPz+5Bgjc aGQw0VuX2GbBPWjoWIZTDfveAaokYS6wIE74K8cxdxBzD2GPvZ/7zRUZ7v1OFVokhDrO 0XPCLHTo5nfRpCUkaCDzS817VIC+Tmwt4cgGNSKHWYTbYhNWtW6GDqwIUKN7Hk8oSoLV lH31REcVHPLBnTeF12J+CIQJhdoyfrBKDcde++rVw/xtm5tO+qDOMVxccIQa9yFpcDXZ kpuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=IQlbAsWIdgnRzHlP1pG9xWwKdhyZdUkCDTuuG+0hh0M=; b=SeviU/IKFlh1DQZcoCS1CTTxjXrXPb+XMiBupx6s9F0qznbEwFOySFSjOnLzUYaIh4 pXhgVUlZxbIJ7aagtLk70n06a5D6zFO99u/muEGoC4XCn1euz1JYA7vNDgkd2CW2ayY6 bmznsZkA97KudmYZhDV4C3DpN7jzy5CpAMnAp4CetWL+rAT7bdDDvojd4ZaGwVSUkBVO 7y5OZBmUDuniAmEEahXylnWTSmkfgU+PKt3+URKlG14gBTx1DI4Gy014v3MkRJFBvFNh qlHuFA6QwKBvLCqlrUp50jR6Y9tWBj2N6fSKRRTY18KOXOwvd3CbHFf87WYJRXQJncfO yJJQ== X-Gm-Message-State: AOPr4FUY89AXr0lZ8peQtaltoVXFo2csiTkEHrpORMU6srFuTY1ZcFgxBvdtB2zt4gPXcw== X-Received: by 10.194.123.65 with SMTP id ly1mr25982914wjb.125.1460794356486; Sat, 16 Apr 2016 01:12:36 -0700 (PDT) Received: from localhost.localdomain ([46.249.74.23]) by smtp.gmail.com with ESMTPSA id e140sm26446801wma.13.2016.04.16.01.12.35 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sat, 16 Apr 2016 01:12:35 -0700 (PDT) From: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> To: sakari.ailus@iki.fi, mchehab@osg.samsung.com Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> Subject: [PATCH] [media] smiapp: provide g_skip_top_lines method in sensor ops Date: Sat, 16 Apr 2016 11:12:20 +0300 Message-Id: <1460794340-490-1-git-send-email-ivo.g.dimitrov.75@gmail.com> X-Mailer: git-send-email 1.9.1 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2016.4.16.80615 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' FORGED_FROM_GMAIL 0.1, MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_2000_2999 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, DKIM_SIGNATURE 0, NO_URI_HTTPS 0, SINGLE_URI_IN_BODY 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __CP_MEDIA_BODY 0, __CP_URI_IN_BODY 0, __DATE_TZ_RU 0, __FRAUD_BODY_WEBMAIL 0, __FRAUD_WEBMAIL 0, __FRAUD_WEBMAIL_FROM 0, __FROM_GMAIL 0, __HAS_FROM 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __PHISH_SPEAR_STRUCTURE_1 0, __SANE_MSGID 0, __SINGLE_URI_TEXT 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_IN_BODY 0, __URI_NO_WWW 0, __URI_NS , __URI_WITH_PATH 0, __YOUTUBE_RCVD 0' |
Commit Message
Ivaylo Dimitrov
April 16, 2016, 8:12 a.m. UTC
Some sensors (like the one in Nokia N900) provide metadata in the first
couple of lines. Make that information information available to the
pipeline.
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
drivers/media/i2c/smiapp/smiapp-core.c | 12 ++++++++++++
drivers/media/i2c/smiapp/smiapp.h | 1 +
2 files changed, 13 insertions(+)
Comments
Hi Ivaylo, On Sat, Apr 16, 2016 at 11:12:20AM +0300, Ivaylo Dimitrov wrote: > Some sensors (like the one in Nokia N900) provide metadata in the first > couple of lines. Make that information information available to the > pipeline. > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > --- > drivers/media/i2c/smiapp/smiapp-core.c | 12 ++++++++++++ > drivers/media/i2c/smiapp/smiapp.h | 1 + > 2 files changed, 13 insertions(+) > > diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c > index a215efe..3dfe387 100644 > --- a/drivers/media/i2c/smiapp/smiapp-core.c > +++ b/drivers/media/i2c/smiapp/smiapp-core.c > @@ -188,6 +188,8 @@ static int smiapp_read_frame_fmt(struct smiapp_sensor *sensor) > embedded_end = 0; > } > > + sensor->image_start = image_start; > + > dev_dbg(&client->dev, "embedded data from lines %d to %d\n", > embedded_start, embedded_end); > dev_dbg(&client->dev, "image data starts at line %d\n", image_start); > @@ -2280,6 +2282,15 @@ static int smiapp_get_skip_frames(struct v4l2_subdev *subdev, u32 *frames) > return 0; > } > > +static int smiapp_get_skip_top_lines(struct v4l2_subdev *subdev, u32 *lines) > +{ > + struct smiapp_sensor *sensor = to_smiapp_sensor(subdev); > + > + *lines = sensor->image_start; > + > + return 0; > +} > + > /* ----------------------------------------------------------------------------- > * sysfs attributes > */ > @@ -2890,6 +2901,7 @@ static const struct v4l2_subdev_pad_ops smiapp_pad_ops = { > > static const struct v4l2_subdev_sensor_ops smiapp_sensor_ops = { > .g_skip_frames = smiapp_get_skip_frames, > + .g_skip_top_lines = smiapp_get_skip_top_lines, > }; > > static const struct v4l2_subdev_ops smiapp_ops = { > diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h > index f6af0cc..c8b4ca0 100644 > --- a/drivers/media/i2c/smiapp/smiapp.h > +++ b/drivers/media/i2c/smiapp/smiapp.h > @@ -217,6 +217,7 @@ struct smiapp_sensor { > > u8 hvflip_inv_mask; /* H/VFLIP inversion due to sensor orientation */ > u8 frame_skip; > + u32 image_start; /* Offset to first line after metadata lines */ > > int power_count; > I'm afraid I think this is not exactly the best way to approach the issue. It'd work, somehow, yes, but --- 1. A compliant sensor (at least in theory) is able to tell this information itself. The number of metadata lines is present in the sensor frame format descriptors. 2. The more generic problem of describing the frame layout should be solved. Sensor metadata is just a special case of this. I've proposed frame descriptors (see an old RFC <URL:http://www.spinics.net/lists/linux-media/msg67295.html>), but this is just a partial solution as well; the APIs would need to be extended to support metadata capture (I think Laurent has been working on that). So a proper solution will require a little bit of time still.
Hi, On 18.04.2016 00:44, Sakari Ailus wrote: > Hi Ivaylo, > > On Sat, Apr 16, 2016 at 11:12:20AM +0300, Ivaylo Dimitrov wrote: >> Some sensors (like the one in Nokia N900) provide metadata in the first >> couple of lines. Make that information information available to the >> pipeline. >> >> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> >> --- >> drivers/media/i2c/smiapp/smiapp-core.c | 12 ++++++++++++ >> drivers/media/i2c/smiapp/smiapp.h | 1 + >> 2 files changed, 13 insertions(+) >> ... > > I'm afraid I think this is not exactly the best way to approach the issue. > It'd work, somehow, yes, but --- > > 1. A compliant sensor (at least in theory) is able to tell this information > itself. The number of metadata lines is present in the sensor frame format > descriptors. > Right. And this is where that number is taken from in the patch and made available to whoever wants to use it. See http://lxr.free-electrons.com/source/drivers/media/i2c/smiapp/smiapp-core.c#L177 . I don't really understand your point here. Maybe the patch description is fuzzy? Could you elaborate? > 2. The more generic problem of describing the frame layout should be solved. > Sensor metadata is just a special case of this. I've proposed frame > descriptors (see an old RFC > <URL:http://www.spinics.net/lists/linux-media/msg67295.html>), but this is > just a partial solution as well; the APIs would need to be extended to > support metadata capture (I think Laurent has been working on that). > Could be, however what we have right now is http://lxr.free-electrons.com/source/drivers/media/platform/omap3isp/ispccp2.c#L369. Also, the patch is not trying to solve the problem with frame format description(or anything in general), but a mere way to pass an already available information in the sensor which is needed by omap3isp, by using an already existing API. I don't see how's that related to the way v4l API going to evolve in some (distant?) future. Not to say that once those frame format descriptors are available, it should be relatively easy to simply remove g_skip_top_lines form v4l2_subdev_sensor_ops and fix the drivers to use the new API. BTW if you have any idea on how to pass (or set) the number of lines to be skipped at the start of the frame to omap3isp driver in some other way, I am fine with dropping the $subject patch and sending another one implementing your proposal. Regards, Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ivaylo, On Mon, Apr 18, 2016 at 09:27:53AM +0300, Ivaylo Dimitrov wrote: > Hi, > > On 18.04.2016 00:44, Sakari Ailus wrote: > >Hi Ivaylo, > > > >On Sat, Apr 16, 2016 at 11:12:20AM +0300, Ivaylo Dimitrov wrote: > >>Some sensors (like the one in Nokia N900) provide metadata in the first > >>couple of lines. Make that information information available to the > >>pipeline. > >> > >>Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > >>--- > >> drivers/media/i2c/smiapp/smiapp-core.c | 12 ++++++++++++ > >> drivers/media/i2c/smiapp/smiapp.h | 1 + > >> 2 files changed, 13 insertions(+) > >> > ... > > > >I'm afraid I think this is not exactly the best way to approach the issue. > >It'd work, somehow, yes, but --- > > > >1. A compliant sensor (at least in theory) is able to tell this information > >itself. The number of metadata lines is present in the sensor frame format > >descriptors. > > > > Right. And this is where that number is taken from in the patch and made > available to whoever wants to use it. See http://lxr.free-electrons.com/source/drivers/media/i2c/smiapp/smiapp-core.c#L177 > . I don't really understand your point here. Maybe the patch description is > fuzzy? Could you elaborate? I missed just that part, apologies for that. I'll apply this into my tree. > > >2. The more generic problem of describing the frame layout should be solved. > >Sensor metadata is just a special case of this. I've proposed frame > >descriptors (see an old RFC > ><URL:http://www.spinics.net/lists/linux-media/msg67295.html>), but this is > >just a partial solution as well; the APIs would need to be extended to > >support metadata capture (I think Laurent has been working on that). > > > > Could be, however what we have right now is http://lxr.free-electrons.com/source/drivers/media/platform/omap3isp/ispccp2.c#L369. > Also, the patch is not trying to solve the problem with frame format > description(or anything in general), but a mere way to pass an already > available information in the sensor which is needed by omap3isp, by using an > already existing API. I don't see how's that related to the way v4l API > going to evolve in some (distant?) future. Not to say that once those frame > format descriptors are available, it should be relatively easy to simply > remove g_skip_top_lines form v4l2_subdev_sensor_ops and fix the drivers to > use the new API. > > BTW if you have any idea on how to pass (or set) the number of lines to be > skipped at the start of the frame to omap3isp driver in some other way, I am > fine with dropping the $subject patch and sending another one implementing > your proposal. Hopefully we'll have a better solution in not so distant future. Metadata is just a special case for frame descriptors.
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c index a215efe..3dfe387 100644 --- a/drivers/media/i2c/smiapp/smiapp-core.c +++ b/drivers/media/i2c/smiapp/smiapp-core.c @@ -188,6 +188,8 @@ static int smiapp_read_frame_fmt(struct smiapp_sensor *sensor) embedded_end = 0; } + sensor->image_start = image_start; + dev_dbg(&client->dev, "embedded data from lines %d to %d\n", embedded_start, embedded_end); dev_dbg(&client->dev, "image data starts at line %d\n", image_start); @@ -2280,6 +2282,15 @@ static int smiapp_get_skip_frames(struct v4l2_subdev *subdev, u32 *frames) return 0; } +static int smiapp_get_skip_top_lines(struct v4l2_subdev *subdev, u32 *lines) +{ + struct smiapp_sensor *sensor = to_smiapp_sensor(subdev); + + *lines = sensor->image_start; + + return 0; +} + /* ----------------------------------------------------------------------------- * sysfs attributes */ @@ -2890,6 +2901,7 @@ static const struct v4l2_subdev_pad_ops smiapp_pad_ops = { static const struct v4l2_subdev_sensor_ops smiapp_sensor_ops = { .g_skip_frames = smiapp_get_skip_frames, + .g_skip_top_lines = smiapp_get_skip_top_lines, }; static const struct v4l2_subdev_ops smiapp_ops = { diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h index f6af0cc..c8b4ca0 100644 --- a/drivers/media/i2c/smiapp/smiapp.h +++ b/drivers/media/i2c/smiapp/smiapp.h @@ -217,6 +217,7 @@ struct smiapp_sensor { u8 hvflip_inv_mask; /* H/VFLIP inversion due to sensor orientation */ u8 frame_skip; + u32 image_start; /* Offset to first line after metadata lines */ int power_count;