You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/10/24 13:17:30 UTC
[GitHub] [incubator-nuttx] SPRESENSE opened a new pull request, #7416: drivers: video: Update some features
SPRESENSE opened a new pull request, #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416
## Summary
Update video driver with isx012 and isx019 image sensor
- Add missing FAR qualifier
- Add get frame interval API
- Update API to dump register for debug
- isx012: Support clip feature
- isx019: Increase waiting time for HW initialization
- isx019: Fix white balance control and capability
- isx019: Fix incorrect JPEG DQT table
## Impact
video driver.
## Testing
spresense:example_camera
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] SPRESENSE commented on a diff in pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on code in PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416#discussion_r1004701473
##########
drivers/video/isx019.c:
##########
@@ -2128,7 +2226,53 @@ static int set_wbmode(imgsensor_value_t val)
isx019_i2c_write(CAT_CATAWB, AWBUSER_NO, (FAR uint8_t *)&toggle, 1);
isx019_i2c_write(CAT_CATAWB, AWBMODE, &mode, 1);
+ return OK;
+}
+
+static bool is_awb_enable(void)
+{
+ uint8_t mode = AWBMODE_AUTO;
+
+ isx019_i2c_read(CAT_CATAWB, AWBMODE, &mode, 1);
+
+ return (mode == AWBMODE_HOLD) ? false : true;
Review Comment:
Fixed.
##########
drivers/video/video.c:
##########
@@ -402,10 +402,10 @@ static video_parameter_name_t g_video_parameter_name[] =
};
static FAR void *video_handler;
-static FAR const struct imgsensor_ops_s **g_video_registered_sensor;
-static int g_video_registered_sensor_num;
-static FAR const struct imgsensor_ops_s *g_video_sensor_ops;
-static FAR const struct imgdata_ops_s *g_video_data_ops;
+static FAR const struct imgsensor_ops_s **g_video_registered_sensor = NULL;
+static int g_video_registered_sensor_num = 0;
+static FAR const struct imgsensor_ops_s *g_video_sensor_ops = NULL;
+static FAR const struct imgdata_ops_s *g_video_data_ops = NULL;
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416#discussion_r1004277736
##########
drivers/video/isx019.c:
##########
@@ -2128,7 +2226,53 @@ static int set_wbmode(imgsensor_value_t val)
isx019_i2c_write(CAT_CATAWB, AWBUSER_NO, (FAR uint8_t *)&toggle, 1);
isx019_i2c_write(CAT_CATAWB, AWBMODE, &mode, 1);
+ return OK;
+}
+
+static bool is_awb_enable(void)
+{
+ uint8_t mode = AWBMODE_AUTO;
+
+ isx019_i2c_read(CAT_CATAWB, AWBMODE, &mode, 1);
+
+ return (mode == AWBMODE_HOLD) ? false : true;
Review Comment:
```suggestion
return mode != AWBMODE_HOLD;
```
##########
drivers/video/isx019.c:
##########
@@ -1503,30 +1528,27 @@ static int isx019_start_capture(imgsensor_stream_type_t type,
}
switch (fmt[IMGSENSOR_FMT_MAIN].width)
- {
- case 1280:
- regval |= FPGA_SCALE_1280_960;
- activate_clip(type,
- fmt[IMGSENSOR_FMT_MAIN].width,
- fmt[IMGSENSOR_FMT_MAIN].height);
- break;
-
- case 640:
- regval |= FPGA_SCALE_640_480;
- activate_clip(type,
- fmt[IMGSENSOR_FMT_MAIN].width,
- fmt[IMGSENSOR_FMT_MAIN].height);
- break;
-
- case 320:
- regval |= FPGA_SCALE_320_240;
- break;
-
- default: /* 160 */
-
- regval |= FPGA_SCALE_160_120;
- break;
- }
+ {
+ case 1280:
+ regval |= FPGA_SCALE_1280_960;
+ break;
+
+ case 640:
+ regval |= FPGA_SCALE_640_480;
+ break;
+
+ case 320:
+ regval |= FPGA_SCALE_320_240;
+ break;
+
+ default: /* 160 */
+ regval |= FPGA_SCALE_160_120;
+ break;
+ }
+
+ activate_clip(type,
+ fmt[IMGSENSOR_FMT_MAIN].width,
+ fmt[IMGSENSOR_FMT_MAIN].height);
Review Comment:
```suggestion
activate_clip(type,
fmt[IMGSENSOR_FMT_MAIN].width,
fmt[IMGSENSOR_FMT_MAIN].height);
```
##########
drivers/video/video.c:
##########
@@ -402,10 +402,10 @@ static video_parameter_name_t g_video_parameter_name[] =
};
static FAR void *video_handler;
-static FAR const struct imgsensor_ops_s **g_video_registered_sensor;
-static int g_video_registered_sensor_num;
-static FAR const struct imgsensor_ops_s *g_video_sensor_ops;
-static FAR const struct imgdata_ops_s *g_video_data_ops;
+static FAR const struct imgsensor_ops_s **g_video_registered_sensor = NULL;
+static int g_video_registered_sensor_num = 0;
+static FAR const struct imgsensor_ops_s *g_video_sensor_ops = NULL;
+static FAR const struct imgdata_ops_s *g_video_data_ops = NULL;
Review Comment:
this is not needed as default zero init is defined by C standard
##########
drivers/video/isx019.c:
##########
@@ -1076,34 +1086,42 @@ static int set_drive_mode(void)
static bool try_repeat(int sec, int usec, CODE int (*trial_func)(void))
{
int ret;
- struct timeval start;
- struct timeval now;
- struct timeval delta;
- struct timeval wait;
+ struct timespec start;
+ struct timespec now;
+ struct timespec delta;
- wait.tv_sec = sec;
- wait.tv_usec = usec;
+ ret = clock_systime_timespec(&start);
+ if (ret < 0)
+ {
+ return false;
+ }
- gettimeofday(&start, NULL);
while (1)
{
ret = trial_func();
- if (ret != -ENODEV)
+ if (ret >= 0)
{
- break;
+ return true;
Review Comment:
the logic seems to be changes hardly here. What are the possible return values for `trial_func()`?
I mean that in current code the wait is executed only of `ret == -ENODEV`, but in proposed code all the negative codes will lead to wait logic. Is that by design?
##########
drivers/video/isx012.c:
##########
@@ -2150,14 +2300,57 @@ static int isx012_get_value(uint32_t id,
return ret;
}
+static bool validate_clip_setting(uint32_t sz, FAR uint32_t *clip)
+{
+ bool ret = false;
+ uint32_t w;
+ uint32_t h;
+
+ DEBUGASSERT(clip);
+
+ if (sz != IMGSENSOR_CLIP_NELEM * sizeof(uint32_t))
+ {
+ return ret;
+ }
+
+ w = clip[IMGSENSOR_CLIP_INDEX_WIDTH];
+ h = clip[IMGSENSOR_CLIP_INDEX_HEIGHT];
+
+ if ((w % CLIP_SIZE_UNIT == 0) && (h % CLIP_SIZE_UNIT == 0))
+ {
+ ret = true;
+ }
+
+ return ret;
+}
+
+static int set_clip(uint32_t size,
+ FAR uint32_t *val,
+ FAR isx012_rect_t *target)
+{
+ DEBUGASSERT(target);
+
+ if (!validate_clip_setting(size, val))
+ {
+ return -EINVAL;
+ }
+
+ target->left = (int32_t)val[IMGSENSOR_CLIP_INDEX_LEFT];
+ target->top = (int32_t)val[IMGSENSOR_CLIP_INDEX_TOP];
+ target->width = val[IMGSENSOR_CLIP_INDEX_WIDTH];
+ target->height = val[IMGSENSOR_CLIP_INDEX_HEIGHT];
+
+ return OK;
+}
+
static int isx012_set_value(uint32_t id,
uint32_t size,
- FAR imgsensor_value_t value)
+ imgsensor_value_t value)
{
FAR struct isx012_dev_s *priv = &g_isx012_private;
int ret = -EINVAL;
uint8_t cnt;
Review Comment:
```suggestion
int ret = -EINVAL;
uint8_t cnt;
```
##########
drivers/video/isx012.c:
##########
@@ -841,6 +988,11 @@ static int isx012_set_mode_param(isx012_dev_t *priv,
uint16_t vsize_addr;
uint8_t smode;
uint8_t mode;
Review Comment:
```suggestion
uint8_t smode;
uint8_t mode;
```
##########
drivers/video/isx019.c:
##########
@@ -1442,6 +1461,12 @@ static int activate_clip(imgsensor_stream_type_t type,
}
break;
+
+ default: /* Otherwise, clear clip setting. */
+ size = FPGA_CLIP_NON;
+ top = 0;
+ left = 0;
+ break;
Review Comment:
```suggestion
left = 0;
break;
```
##########
drivers/video/isx012.c:
##########
@@ -825,7 +845,134 @@ static bool is_movie_needed(uint8_t fmt, uint8_t fps)
return need;
}
-static int isx012_set_mode_param(isx012_dev_t *priv,
+static void resize_for_clip(uint8_t nr_fmt,
+ FAR imgsensor_format_t *fmt,
+ FAR isx012_rect_t *clip,
+ FAR uint16_t *w,
+ FAR uint16_t *h,
+ FAR uint16_t *s_w,
+ FAR uint16_t *s_h)
+{
+ ASSERT(fmt && clip && w && h && s_w && s_h);
+
+ *w = (clip->width == 0) ? fmt[IMGSENSOR_FMT_MAIN].width : clip->width;
+ *h = (clip->height == 0) ? fmt[IMGSENSOR_FMT_MAIN].height : clip->height;
+
+ if (nr_fmt > 1)
+ {
+ *s_w = fmt[IMGSENSOR_FMT_SUB].width;
+ if (clip->width > 0)
+ {
+ *s_w = (uint32_t)*s_w * clip->width /
+ fmt[IMGSENSOR_FMT_MAIN].width;
+ }
+
+ *s_h = fmt[IMGSENSOR_FMT_SUB].height;
+ if (clip->height > 0)
+ {
+ *s_h = (uint32_t)*s_h * clip->height /
+ fmt[IMGSENSOR_FMT_MAIN].height;
+ }
+ }
+}
+
+static void calc_clip_regval(uint16_t pos,
+ uint16_t clip_sz,
+ uint16_t frm_sz,
+ uint16_t basis_sz,
+ FAR uint32_t *ratio,
+ FAR int32_t *offset)
+{
+ DEBUGASSERT(ratio && offset);
+
+ *ratio = ZOOM_UNIT;
+ *offset = 0;
+
+ if (clip_sz != 0)
+ {
+ /* Clip by setting zoom up. */
+
+ *ratio *= frm_sz;
+ *ratio /= clip_sz;
+
+ /* Applications' request pos means position from the upper left corner.
+ * On the other hand, ISX012's register means the center of the image,
+ * which has the maximum size of the sensor.
+ */
+
+ *offset = CLIP_OFFSET_UNIT;
+ *offset *= (int16_t)(pos + (clip_sz / 2) - (frm_sz / 2));
+ *offset *= basis_sz;
+ *offset /= (int32_t)frm_sz;
+ }
+}
+
+static bool is_clipped(FAR isx012_rect_t *clip)
+{
+ DEBUGASSERT(clip);
+
+ if ((clip->left == 0) &&
+ (clip->top == 0) &&
+ (clip->width == 0) &&
+ (clip->height == 0))
+ {
+ return false;
+ }
+
+ return true;
+}
+
+static void activate_clip(FAR isx012_dev_t *priv,
+ uint16_t w,
+ uint16_t h,
+ FAR isx012_rect_t *clip)
+{
+ uint8_t hvfree = 0;
+ uint32_t r_x = ZOOM_UNIT;
+ uint32_t r_y = ZOOM_UNIT;
+ int32_t x = 0;
+ int32_t y = 0;
Review Comment:
Minor, optional: either
```
uint8_t hvfree = 0;
uint32_t r_x = ZOOM_UNIT;
uint32_t r_y = ZOOM_UNIT;
int32_t x = 0;
int32_t y = 0;
```
or
```
uint8_t hvfree = 0;
uint32_t r_x = ZOOM_UNIT;
uint32_t r_y = ZOOM_UNIT;
int32_t x = 0;
int32_t y = 0;
```
##########
drivers/video/isx012.c:
##########
@@ -1827,7 +1977,7 @@ static int isx012_get_value(uint32_t id,
uint8_t cnt;
uint8_t threea_enable;
uint16_t read_src;
- uint16_t *read_dst;
+ FAR uint16_t *read_dst;
int ret = OK;
uint16_t exposure_time_lsb;
uint16_t exposure_time_msb;
Review Comment:
```suggestion
int ret = OK;
uint16_t exposure_time_lsb;
uint16_t exposure_time_msb;
```
##########
drivers/video/isx012.c:
##########
@@ -2150,14 +2300,57 @@ static int isx012_get_value(uint32_t id,
return ret;
}
+static bool validate_clip_setting(uint32_t sz, FAR uint32_t *clip)
+{
+ bool ret = false;
+ uint32_t w;
+ uint32_t h;
+
+ DEBUGASSERT(clip);
+
+ if (sz != IMGSENSOR_CLIP_NELEM * sizeof(uint32_t))
+ {
+ return ret;
+ }
+
+ w = clip[IMGSENSOR_CLIP_INDEX_WIDTH];
+ h = clip[IMGSENSOR_CLIP_INDEX_HEIGHT];
+
+ if ((w % CLIP_SIZE_UNIT == 0) && (h % CLIP_SIZE_UNIT == 0))
+ {
+ ret = true;
+ }
+
+ return ret;
+}
+
+static int set_clip(uint32_t size,
+ FAR uint32_t *val,
+ FAR isx012_rect_t *target)
+{
+ DEBUGASSERT(target);
+
+ if (!validate_clip_setting(size, val))
+ {
+ return -EINVAL;
+ }
+
+ target->left = (int32_t)val[IMGSENSOR_CLIP_INDEX_LEFT];
+ target->top = (int32_t)val[IMGSENSOR_CLIP_INDEX_TOP];
+ target->width = val[IMGSENSOR_CLIP_INDEX_WIDTH];
+ target->height = val[IMGSENSOR_CLIP_INDEX_HEIGHT];
+
+ return OK;
+}
+
static int isx012_set_value(uint32_t id,
uint32_t size,
- FAR imgsensor_value_t value)
+ imgsensor_value_t value)
{
FAR struct isx012_dev_s *priv = &g_isx012_private;
int ret = -EINVAL;
uint8_t cnt;
- uint16_t *write_src;
+ FAR uint16_t *write_src;
uint16_t write_dst;
uint16_t regval;
uint16_t exposure_time_lsb;
Review Comment:
```suggestion
uint16_t write_dst;
uint16_t regval;
uint16_t exposure_time_lsb;
```
##########
drivers/video/isx012.c:
##########
@@ -1827,7 +1977,7 @@ static int isx012_get_value(uint32_t id,
uint8_t cnt;
uint8_t threea_enable;
uint16_t read_src;
Review Comment:
```suggestion
uint8_t cnt;
uint8_t threea_enable;
uint16_t read_src;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] SPRESENSE commented on a diff in pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on code in PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416#discussion_r1004703524
##########
drivers/video/isx012.c:
##########
@@ -2150,14 +2300,57 @@ static int isx012_get_value(uint32_t id,
return ret;
}
+static bool validate_clip_setting(uint32_t sz, FAR uint32_t *clip)
+{
+ bool ret = false;
+ uint32_t w;
+ uint32_t h;
+
+ DEBUGASSERT(clip);
+
+ if (sz != IMGSENSOR_CLIP_NELEM * sizeof(uint32_t))
+ {
+ return ret;
+ }
+
+ w = clip[IMGSENSOR_CLIP_INDEX_WIDTH];
+ h = clip[IMGSENSOR_CLIP_INDEX_HEIGHT];
+
+ if ((w % CLIP_SIZE_UNIT == 0) && (h % CLIP_SIZE_UNIT == 0))
+ {
+ ret = true;
+ }
+
+ return ret;
+}
+
+static int set_clip(uint32_t size,
+ FAR uint32_t *val,
+ FAR isx012_rect_t *target)
+{
+ DEBUGASSERT(target);
+
+ if (!validate_clip_setting(size, val))
+ {
+ return -EINVAL;
+ }
+
+ target->left = (int32_t)val[IMGSENSOR_CLIP_INDEX_LEFT];
+ target->top = (int32_t)val[IMGSENSOR_CLIP_INDEX_TOP];
+ target->width = val[IMGSENSOR_CLIP_INDEX_WIDTH];
+ target->height = val[IMGSENSOR_CLIP_INDEX_HEIGHT];
+
+ return OK;
+}
+
static int isx012_set_value(uint32_t id,
uint32_t size,
- FAR imgsensor_value_t value)
+ imgsensor_value_t value)
{
FAR struct isx012_dev_s *priv = &g_isx012_private;
int ret = -EINVAL;
uint8_t cnt;
Review Comment:
Fixed.
##########
drivers/video/isx012.c:
##########
@@ -1827,7 +1977,7 @@ static int isx012_get_value(uint32_t id,
uint8_t cnt;
uint8_t threea_enable;
uint16_t read_src;
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] SPRESENSE commented on a diff in pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on code in PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416#discussion_r1004701232
##########
drivers/video/isx019.c:
##########
@@ -1503,30 +1528,27 @@ static int isx019_start_capture(imgsensor_stream_type_t type,
}
switch (fmt[IMGSENSOR_FMT_MAIN].width)
- {
- case 1280:
- regval |= FPGA_SCALE_1280_960;
- activate_clip(type,
- fmt[IMGSENSOR_FMT_MAIN].width,
- fmt[IMGSENSOR_FMT_MAIN].height);
- break;
-
- case 640:
- regval |= FPGA_SCALE_640_480;
- activate_clip(type,
- fmt[IMGSENSOR_FMT_MAIN].width,
- fmt[IMGSENSOR_FMT_MAIN].height);
- break;
-
- case 320:
- regval |= FPGA_SCALE_320_240;
- break;
-
- default: /* 160 */
-
- regval |= FPGA_SCALE_160_120;
- break;
- }
+ {
+ case 1280:
+ regval |= FPGA_SCALE_1280_960;
+ break;
+
+ case 640:
+ regval |= FPGA_SCALE_640_480;
+ break;
+
+ case 320:
+ regval |= FPGA_SCALE_320_240;
+ break;
+
+ default: /* 160 */
+ regval |= FPGA_SCALE_160_120;
+ break;
+ }
+
+ activate_clip(type,
+ fmt[IMGSENSOR_FMT_MAIN].width,
+ fmt[IMGSENSOR_FMT_MAIN].height);
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] SPRESENSE commented on a diff in pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on code in PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416#discussion_r1004704105
##########
drivers/video/isx012.c:
##########
@@ -825,7 +845,134 @@ static bool is_movie_needed(uint8_t fmt, uint8_t fps)
return need;
}
-static int isx012_set_mode_param(isx012_dev_t *priv,
+static void resize_for_clip(uint8_t nr_fmt,
+ FAR imgsensor_format_t *fmt,
+ FAR isx012_rect_t *clip,
+ FAR uint16_t *w,
+ FAR uint16_t *h,
+ FAR uint16_t *s_w,
+ FAR uint16_t *s_h)
+{
+ ASSERT(fmt && clip && w && h && s_w && s_h);
+
+ *w = (clip->width == 0) ? fmt[IMGSENSOR_FMT_MAIN].width : clip->width;
+ *h = (clip->height == 0) ? fmt[IMGSENSOR_FMT_MAIN].height : clip->height;
+
+ if (nr_fmt > 1)
+ {
+ *s_w = fmt[IMGSENSOR_FMT_SUB].width;
+ if (clip->width > 0)
+ {
+ *s_w = (uint32_t)*s_w * clip->width /
+ fmt[IMGSENSOR_FMT_MAIN].width;
+ }
+
+ *s_h = fmt[IMGSENSOR_FMT_SUB].height;
+ if (clip->height > 0)
+ {
+ *s_h = (uint32_t)*s_h * clip->height /
+ fmt[IMGSENSOR_FMT_MAIN].height;
+ }
+ }
+}
+
+static void calc_clip_regval(uint16_t pos,
+ uint16_t clip_sz,
+ uint16_t frm_sz,
+ uint16_t basis_sz,
+ FAR uint32_t *ratio,
+ FAR int32_t *offset)
+{
+ DEBUGASSERT(ratio && offset);
+
+ *ratio = ZOOM_UNIT;
+ *offset = 0;
+
+ if (clip_sz != 0)
+ {
+ /* Clip by setting zoom up. */
+
+ *ratio *= frm_sz;
+ *ratio /= clip_sz;
+
+ /* Applications' request pos means position from the upper left corner.
+ * On the other hand, ISX012's register means the center of the image,
+ * which has the maximum size of the sensor.
+ */
+
+ *offset = CLIP_OFFSET_UNIT;
+ *offset *= (int16_t)(pos + (clip_sz / 2) - (frm_sz / 2));
+ *offset *= basis_sz;
+ *offset /= (int32_t)frm_sz;
+ }
+}
+
+static bool is_clipped(FAR isx012_rect_t *clip)
+{
+ DEBUGASSERT(clip);
+
+ if ((clip->left == 0) &&
+ (clip->top == 0) &&
+ (clip->width == 0) &&
+ (clip->height == 0))
+ {
+ return false;
+ }
+
+ return true;
+}
+
+static void activate_clip(FAR isx012_dev_t *priv,
+ uint16_t w,
+ uint16_t h,
+ FAR isx012_rect_t *clip)
+{
+ uint8_t hvfree = 0;
+ uint32_t r_x = ZOOM_UNIT;
+ uint32_t r_y = ZOOM_UNIT;
+ int32_t x = 0;
+ int32_t y = 0;
Review Comment:
Fixed.
##########
drivers/video/isx019.c:
##########
@@ -1442,6 +1461,12 @@ static int activate_clip(imgsensor_stream_type_t type,
}
break;
+
+ default: /* Otherwise, clear clip setting. */
+ size = FPGA_CLIP_NON;
+ top = 0;
+ left = 0;
+ break;
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] SPRESENSE commented on a diff in pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on code in PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416#discussion_r1004703029
##########
drivers/video/isx019.c:
##########
@@ -1076,34 +1086,42 @@ static int set_drive_mode(void)
static bool try_repeat(int sec, int usec, CODE int (*trial_func)(void))
{
int ret;
- struct timeval start;
- struct timeval now;
- struct timeval delta;
- struct timeval wait;
+ struct timespec start;
+ struct timespec now;
+ struct timespec delta;
- wait.tv_sec = sec;
- wait.tv_usec = usec;
+ ret = clock_systime_timespec(&start);
+ if (ret < 0)
+ {
+ return false;
+ }
- gettimeofday(&start, NULL);
while (1)
{
ret = trial_func();
- if (ret != -ENODEV)
+ if (ret >= 0)
{
- break;
+ return true;
Review Comment:
Yes, the logic has changed, but there is no particular problem with the updated code.
##########
drivers/video/isx012.c:
##########
@@ -2150,14 +2300,57 @@ static int isx012_get_value(uint32_t id,
return ret;
}
+static bool validate_clip_setting(uint32_t sz, FAR uint32_t *clip)
+{
+ bool ret = false;
+ uint32_t w;
+ uint32_t h;
+
+ DEBUGASSERT(clip);
+
+ if (sz != IMGSENSOR_CLIP_NELEM * sizeof(uint32_t))
+ {
+ return ret;
+ }
+
+ w = clip[IMGSENSOR_CLIP_INDEX_WIDTH];
+ h = clip[IMGSENSOR_CLIP_INDEX_HEIGHT];
+
+ if ((w % CLIP_SIZE_UNIT == 0) && (h % CLIP_SIZE_UNIT == 0))
+ {
+ ret = true;
+ }
+
+ return ret;
+}
+
+static int set_clip(uint32_t size,
+ FAR uint32_t *val,
+ FAR isx012_rect_t *target)
+{
+ DEBUGASSERT(target);
+
+ if (!validate_clip_setting(size, val))
+ {
+ return -EINVAL;
+ }
+
+ target->left = (int32_t)val[IMGSENSOR_CLIP_INDEX_LEFT];
+ target->top = (int32_t)val[IMGSENSOR_CLIP_INDEX_TOP];
+ target->width = val[IMGSENSOR_CLIP_INDEX_WIDTH];
+ target->height = val[IMGSENSOR_CLIP_INDEX_HEIGHT];
+
+ return OK;
+}
+
static int isx012_set_value(uint32_t id,
uint32_t size,
- FAR imgsensor_value_t value)
+ imgsensor_value_t value)
{
FAR struct isx012_dev_s *priv = &g_isx012_private;
int ret = -EINVAL;
uint8_t cnt;
- uint16_t *write_src;
+ FAR uint16_t *write_src;
uint16_t write_dst;
uint16_t regval;
uint16_t exposure_time_lsb;
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jerpelea commented on pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
jerpelea commented on PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416#issuecomment-1296736816
@SPRESENSE can you please do the changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
acassis commented on PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416#issuecomment-1289464160
Please fix the CI reported error:
```
drivers/video/isx019.c:1109:38: error: Operator/assignment must be followed with whitespace
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] SPRESENSE commented on a diff in pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on code in PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416#discussion_r1004703784
##########
drivers/video/isx012.c:
##########
@@ -1827,7 +1977,7 @@ static int isx012_get_value(uint32_t id,
uint8_t cnt;
uint8_t threea_enable;
uint16_t read_src;
- uint16_t *read_dst;
+ FAR uint16_t *read_dst;
int ret = OK;
uint16_t exposure_time_lsb;
uint16_t exposure_time_msb;
Review Comment:
Fixed.
##########
drivers/video/isx012.c:
##########
@@ -841,6 +988,11 @@ static int isx012_set_mode_param(isx012_dev_t *priv,
uint16_t vsize_addr;
uint8_t smode;
uint8_t mode;
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] SPRESENSE commented on pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416#issuecomment-1290246800
I addd a commit changed not to use `timercmp()` that caused the CI error. please review again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7416: drivers: video: Update some features
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7416:
URL: https://github.com/apache/incubator-nuttx/pull/7416#discussion_r1004752632
##########
drivers/video/isx019.c:
##########
@@ -1076,34 +1086,42 @@ static int set_drive_mode(void)
static bool try_repeat(int sec, int usec, CODE int (*trial_func)(void))
{
int ret;
- struct timeval start;
- struct timeval now;
- struct timeval delta;
- struct timeval wait;
+ struct timespec start;
+ struct timespec now;
+ struct timespec delta;
- wait.tv_sec = sec;
- wait.tv_usec = usec;
+ ret = clock_systime_timespec(&start);
+ if (ret < 0)
+ {
+ return false;
+ }
- gettimeofday(&start, NULL);
while (1)
{
ret = trial_func();
- if (ret != -ENODEV)
+ if (ret >= 0)
{
- break;
+ return true;
Review Comment:
Ok. My only concern was if we need to wait if other than no dev error code is received. But seems to be not a big deal here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org