omap34xxcam question?
Hi
Is ok that it try only the first format and size? why does it not continue and find a matching?
@@ -470,7 +471,7 @@ static int try_pix_parm(struct omap34xxcam_videodev *vdev,
pix_tmp_out = *wanted_pix_out;
rval = isp_try_fmt_cap(isp, &pix_tmp_in, &pix_tmp_out);
if (rval)- return rval;
+ continue;
Michael
Quoted 9 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 6:01 AM
To: [email protected]
Cc: Aguirre, Sergio
Subject: omap34xxcam question?
Quoted 4 lines
Hi
Is ok that it try only the first format and size? why does it not continue and find a matching?
Actually, that was the intention, but I guess it was badly implemented.
Thanks for the catch, and the contribution!
Regards,
Sergio
Quoted 11 lines
(quoted diff) +1
-1@@ -470,7 +471,7 @@ static int try_pix_parm(struct omap34xxcam_videodev *vdev, pix_tmp_out = *wanted_pix_out; rval = isp_try_fmt_cap(isp, &pix_tmp_in, &pix_tmp_out); if (rval) - return rval; + continue;Michael
Hi,
Quoted 19 lines · Sergio
Quoted 8 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 6:01 AM
To: [email protected]
Cc: Aguirre, Sergio
Subject: omap34xxcam question?Quoted 4 lines
Hi
Is ok that it try only the first format and size? why does it not continue and find a matching?
Actually, that was the intention, but I guess it was badly implemented.
Thanks for the catch, and the contribution!
Regards,
Sergio
Quoted 9 lines
Review static int try_pix_parm(struct omap34xxcam_videodev · line 477
Quoted 9 lines
(quoted diff) +1
-1@@ -470,7 +471,7 @@ static int try_pix_parm(struct omap34xxcam_videodev *vdev, pix_tmp_out = *wanted_pix_out; rval = isp_try_fmt_cap(isp, &pix_tmp_in, &pix_tmp_out); if (rval) - return rval; + continue;
Is the patch good? or you are going to provide a better fix
Michael
Quoted 6 lines
Quoted 1 line
Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 32 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 11:25 AM
To: Aguirre, Sergio
Cc: [email protected]
Subject: Re: omap34xxcam question?Hi,
Quoted 11 lines · Sergio
Quoted 11 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 6:01 AM
To: [email protected]
Cc: Aguirre, Sergio
Subject: omap34xxcam question?Hi
Is ok that it try only the first format and size? why does it not
continue
Quoted 8 lines
Quoted 1 line
and find a matching?
Actually, that was the intention, but I guess it was badly implemented.
Thanks for the catch, and the contribution!
Regards,
Sergio
Quoted 11 lines
Quoted 9 lines
Review static int try_pix_parm(struct omap34xxcam_videodev · line 477
Quoted 9 lines
(quoted diff) +1
-1@@ -470,7 +471,7 @@ static int try_pix_parm(struct omap34xxcam_videodev *vdev, pix_tmp_out = *wanted_pix_out; rval = isp_try_fmt_cap(isp, &pix_tmp_in, &pix_tmp_out); if (rval) - return rval; + continue;Is the patch good? or you are going to provide a better fix
Yes. Sorry if I wasn't clear enough.
Looks good to me, and I don't have a better fix on top of my head for the moment...
I'm assuming you tested it in your environment, right?
If yes, then I'll take the patch in my queue for submission to Sakari's tree.
Thanks for your time.
Regards,
Sergio
Quoted 10 lines
Michael
Quoted 3 lines
Quoted 1 line
Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-media"in
Quoted 3 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 40 lines · Sergio
Quoted 29 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 11:25 AM
To: Aguirre, Sergio
Cc: [email protected]
Subject: Re: omap34xxcam question?Hi,
Quoted 10 lines · Sergio
Quoted 10 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 6:01 AM
To: [email protected]
Cc: Aguirre, Sergio
Subject: omap34xxcam question?Hi
Is ok that it try only the first format and size? why does it not
continue
Quoted 7 lines
Quoted 1 line
and find a matching?
Actually, that was the intention, but I guess it was badly implemented.
Thanks for the catch, and the contribution!
Regards,
SergioQuoted 10 lines
Quoted 9 lines
Review static int try_pix_parm(struct omap34xxcam_videodev · line 477
Quoted 9 lines
(quoted diff) +1
-1@@ -470,7 +471,7 @@ static int try_pix_parm(struct omap34xxcam_videodev *vdev, pix_tmp_out = *wanted_pix_out; rval = isp_try_fmt_cap(isp, &pix_tmp_in, &pix_tmp_out); if (rval) - return rval; + continue;Is the patch good? or you are going to provide a better fix
Quoted 5 lines
Yes. Sorry if I wasn't clear enough.
Looks good to me, and I don't have a better fix on top of my head for the moment...
I'm assuming you tested it in your environment, right?
Ok, my enviroment is not pretty stable but for sure this is required. There is one problem:
Suppose that the camera support this format:
YUV and RAW10
The video4linux enumeration is done in this order. We know that if you want to use resizer and previewer we can't use the YUV (go straight to memory) but it is selected because is the first. So maybe the best thing is to find the one that is suggest in the csi configuration first. Hope that is clear.
Michael
Quoted 23 lines
If yes, then I'll take the patch in my queue for submission to Sakari's tree.
Thanks for your time.
Regards,
SergioQuoted 9 lines
Michael
Quoted 3 lines
Quoted 1 line
Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-media"in
Quoted 3 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC PATCH 1/1 fix try_pix_parm loop
This patch fix try_pix_parm loop that try to find a suitable format for the isp engine
---
drivers/media/video/omap34xxcam.c +1 -1
@@ -470,7 +470,7 @@ static int try_pix_parm(struct omap34xxcam_videodev *vdev, pix_tmp_out = *wanted_pix_out; rval =isp_try_fmt_cap (isp, &pix_tmp_in, &pix_tmp_out);if (rval) -return rval; +continue ;dev_dbg (&vdev->vfd->dev,"this w %d\th %d\tfmt %8.8x\t" "-> w %d\th %d\t fmt %8.8x"
Quoted 64 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 11:39 AM
To: Aguirre, Sergio
Cc: [email protected]
Subject: Re: omap34xxcam question?Quoted 18 lines · Sergio
Quoted 18 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 11:25 AM
To: Aguirre, Sergio
Cc: [email protected]
Subject: Re: omap34xxcam question?Hi,
Quoted 7 lines · Sergio
Quoted 7 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 6:01 AM
To: [email protected]
Cc: Aguirre, Sergio
Subject: omap34xxcam question?Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Hi
Is ok that it try only the first format and size? why does it not
continue
Quoted 2 lines
Quoted 1 line
and find a matching?
Actually, that was the intention, but I guess it was badly
implemented.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Thanks for the catch, and the contribution!
Regards,
SergioReview static int try_pix_parm(struct
Quoted 1 line
(quoted diff) +0
-0@@ -470,7 +471,7 @@ static int try_pix_parm(struct
omap34xxcam_videodevQuoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
(quoted diff) +1
-1*vdev, pix_tmp_out = *wanted_pix_out; rval = isp_try_fmt_cap(isp, &pix_tmp_in, &pix_tmp_out); if (rval) - return rval; + continue;Quoted 5 lines
Quoted 1 line
Is the patch good? or you are going to provide a better fix
Yes. Sorry if I wasn't clear enough.
Looks good to me, and I don't have a better fix on top of my head for
the moment...
Quoted 2 lines
I'm assuming you tested it in your environment, right?
Ok, my enviroment is not pretty stable but for sure this is required. There is one problem:
Suppose that the camera support this format:
Quoted 8 lines
YUV and RAW10
The video4linux enumeration is done in this order.
We know that if you want to use resizer and previewer we can't use the YUV (go straight to memory)
but it is selected because is the first. So maybe the best thing is to find the one that is suggest in the csi
configuration first. Hope that is clear.
Hmm.. I see.
So, if I got you right, you're saying that, there should be priorities for sensor baseformats, depending on the preference specified somewhere in the boardfile?
Regards,
Sergio
Quoted 28 lines
Michael
Quoted 2 lines
If yes, then I'll take the patch in my queue for submission to Sakari's
tree.
Quoted 11 lines
Thanks for your time.
Regards,
SergioQuoted 5 lines
Michael
Quoted 3 lines
Quoted 1 line
Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-media"Quoted 7 lines
Quoted 4 lines
in
Quoted 3 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-media"in
Quoted 3 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 67 lines · Sergio
Quoted 58 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 11:39 AM
To: Aguirre, Sergio
Cc: [email protected]
Subject: Re: omap34xxcam question?Quoted 17 lines · Sergio
Quoted 17 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 11:25 AM
To: Aguirre, Sergio
Cc: [email protected]
Subject: Re: omap34xxcam question?Hi,
Quoted 7 lines · Sergio
Quoted 7 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 6:01 AM
To: [email protected]
Cc: Aguirre, Sergio
Subject: omap34xxcam question?Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Hi
Is ok that it try only the first format and size? why does it not
continue
Quoted 2 lines
Quoted 1 line
and find a matching?
Actually, that was the intention, but I guess it was badly
implemented.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Thanks for the catch, and the contribution!
Regards,
SergioReview static int try_pix_parm(struct
Quoted 1 line
(quoted diff) +0
-0@@ -470,7 +471,7 @@ static int try_pix_parm(struct
omap34xxcam_videodevQuoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
(quoted diff) +1
-1*vdev, pix_tmp_out = *wanted_pix_out; rval = isp_try_fmt_cap(isp, &pix_tmp_in, &pix_tmp_out); if (rval) - return rval; + continue;Quoted 4 lines
Quoted 1 line
Is the patch good? or you are going to provide a better fix
Yes. Sorry if I wasn't clear enough.
Looks good to me, and I don't have a better fix on top of my head for
the moment...
Quoted 1 line
I'm assuming you tested it in your environment, right?
Ok, my enviroment is not pretty stable but for sure this is required. There is one problem:
Suppose that the camera support this format:
Quoted 8 lines
YUV and RAW10
The video4linux enumeration is done in this order.
We know that if you want to use resizer and previewer we can't use the YUV (go straight to memory)
but it is selected because is the first. So maybe the best thing is to find the one that is suggest in the csi
configuration first. Hope that is clear.
Quoted 3 lines
Hmm.. I see.
So, if I got you right, you're saying that, there should be priorities for sensor baseformats, depending on the preference specified somewhere in the boardfile?
Yes, that is the idea. Try to provide a better patch later, I'm working hard on the sensor part :)
Michael
Quoted 33 lines
Regards,
SergioQuoted 14 lines
Michael
Quoted 1 line
If yes, then I'll take the patch in my queue for submission to Sakari's
tree.
Quoted 10 lines
Thanks for your time.
Regards,
SergioQuoted 5 lines
Michael
Quoted 3 lines
Quoted 1 line
Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-media"Quoted 10 lines
Quoted 6 lines
Quoted 4 lines
in
Quoted 3 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-media"in
Quoted 3 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 68 lines · Michael Trimarchi
Quoted 67 lines · Sergio
Quoted 66 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 11:39 AM
To: Aguirre, Sergio
Cc: [email protected]
Subject: Re: omap34xxcam question?Quoted 17 lines · Sergio
Quoted 17 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 11:25 AM
To: Aguirre, Sergio
Cc: [email protected]
Subject: Re: omap34xxcam question?Hi,
Quoted 7 lines · Sergio
Quoted 7 lines
-----Original Message-----
From: Michael Trimarchi [mailto:[email protected]]
Sent: Thursday, January 14, 2010 6:01 AM
To: [email protected]
Cc: Aguirre, Sergio
Subject: omap34xxcam question?Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Hi
Is ok that it try only the first format and size? why does it not
continue
Quoted 2 lines
Quoted 1 line
and find a matching?
Actually, that was the intention, but I guess it was badly
implemented.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Thanks for the catch, and the contribution!
Regards,
SergioReview static int try_pix_parm(struct
Quoted 1 line
(quoted diff) +0
-0@@ -470,7 +471,7 @@ static int try_pix_parm(struct
omap34xxcam_videodevQuoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
(quoted diff) +1
-1*vdev, pix_tmp_out = *wanted_pix_out; rval = isp_try_fmt_cap(isp, &pix_tmp_in, &pix_tmp_out); if (rval) - return rval; + continue;Quoted 4 lines
Quoted 1 line
Is the patch good? or you are going to provide a better fix
Yes. Sorry if I wasn't clear enough.
Looks good to me, and I don't have a better fix on top of my head for
the moment...
Quoted 1 line
I'm assuming you tested it in your environment, right?
Ok, my enviroment is not pretty stable but for sure this is required. There is one problem:
Suppose that the camera support this format:
YUV and RAW10
The video4linux enumeration is done in this order.
We know that if you want to use resizer and previewer we can't use the YUV (go straight to memory)
but it is selected because is the first. So maybe the best thing is to find the one that is suggest in the csi
configuration first. Hope that is clear.
Quoted 6 lines
Quoted 3 lines
Hmm.. I see.
So, if I got you right, you're saying that, there should be priorities for sensor baseformats, depending on the preference specified somewhere in the boardfile?
Yes, that is the idea. Try to provide a better patch later, I'm working hard on the sensor part :)
drivers/media/video/omap34xxcam.c +5 -1
@@ -448,6 +448,10 @@ static int try_pix_parm(struct omap34xxcam_videodev *vdev,break ;dev_dbg (&vdev->vfd->dev,"trying fmt %8.8x (%d)\n" , fmtd.pixelformat, fmtd_index); + +if (fmtd.pixelformat != best_pix_in->pixelformat) +continue ; +/* * Get supported resolutions. */
@@ -470,7 +474,7 @@ static int try_pix_parm(struct omap34xxcam_videodev *vdev, pix_tmp_out = *wanted_pix_out; rval =isp_try_fmt_cap (isp, &pix_tmp_in, &pix_tmp_out);if (rval) -return rval; +continue ; dev_dbg(&vdev->vfd->dev, "this w %d\th %d\tfmt %8.8x\t"
Somenthing like that.
michael
Quoted 41 lines
Michael
Quoted 33 lines
Regards,
SergioQuoted 24 lines
Michael
Quoted 1 line
If yes, then I'll take the patch in my queue for submission to Sakari's
tree.
Quoted 7 lines
Thanks for your time.
Regards,
SergioQuoted 2 lines
Michael
Quoted 9 lines
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Michael
Signature · 1 line
To unsubscribe from this list: send the line "unsubscribe linux-media"in
Quoted 3 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.htmlSignature · 1 line
To unsubscribe from this list: send the line "unsubscribe linux-media"in
Quoted 3 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.htmlSignature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.htmlSignature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Michael,
Quoted 1 line · Michael Trimarchi
Aguirre, Sergio wrote:
...
Quoted 6 lines
Quoted 3 lines
So, if I got you right, you're saying that, there should be priorities for sensor baseformats, depending on the preference specified somewhere in the boardfile?
Yes, that is the idea. Try to provide a better patch later, I'm working hard on the sensor part :)
Apologies for my late answer.
The frame sizes in our sensor drivers are in width descending order. The selection has been working somehow so far but it's definitely not perfect.
We're converting the ISP driver to use the Media controller so this code will be dropped in near future probably. In that case the user space has to select the sensor mode it wants to use as well.
Regular V4L2 applications of course cannot be expected to do that. But it probably should be handled in user space (i.e. libv4l) or by offering a dummy video node that just produces some kind of images.
Signature · 2 lines
Sakari Ailus [email protected]
Hi,
Quoted 4 lines · Sakari Ailus
Hi Michael,
Quoted 1 line · Michael Trimarchi
Aguirre, Sergio wrote:
Quoted 8 lines
...
Quoted 5 lines
Quoted 3 lines
So, if I got you right, you're saying that, there should be priorities for sensor baseformats, depending on the preference specified somewhere in the boardfile?
Yes, that is the idea. Try to provide a better patch later, I'm working hard on the sensor part :)
Apologies for my late answer.
No problem on that
Quoted 4 lines
The frame sizes in our sensor drivers are in width descending order. The selection has been working somehow so far but it's definitely not perfect.
Ok for the frame size but you need to test all the possible sensor output too and continue in case of error.
Quoted 4 lines
We're converting the ISP driver to use the Media controller so this code will be dropped in near future probably. In that case the user space has to select the sensor mode it wants to use as well.
Good.
Maybe I can test the framework on the FLOW1.5 mobile device using the TCM8240MD
What is your git for the camera framework?
Michael
Quoted 1 line · Michael Trimarchi
Hi,
Hi,
Quoted 2 lines
Quoted 1 line · Sakari Ailus
Hi Michael,
...
Quoted 7 lines
Quoted 4 lines
The frame sizes in our sensor drivers are in width descending order. The selection has been working somehow so far but it's definitely not perfect.
Ok for the frame size but you need to test all the possible sensor output too and continue in case of error.
The try function should return an error only in real error cases, not otherwise.
But as I said, this code will be removed soon from that driver...
Quoted 11 lines
Quoted 4 lines
We're converting the ISP driver to use the Media controller so this code will be dropped in near future probably. In that case the user space has to select the sensor mode it wants to use as well.
Good.
Maybe I can test the framework on the FLOW1.5 mobile device using the TCM8240MD
What is your git for the camera framework?
It's all available here but I'm afraid you'll have to wait a bit first. The legacy nodes (per sensor devices) are the only ones in existence at the moment.
<URL:http://www.gitorious.org/projects/omap3camera>
Signature · 2 lines
Sakari Ailus [email protected]