Hi Sakari,

Thank you for the patch.

On Monday 15 December 2014 18:26:47 Sakari Ailus wrote:
> Support data_offset for multi plane buffers. Also add an option to write the
> data in the buffer before data offset (--buffer-prefix).
> 
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> ---
>  yavta.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/yavta.c b/yavta.c
> index 77e5a41..003d6ba 100644
> --- a/yavta.c
> +++ b/yavta.c
> @@ -80,6 +80,8 @@ struct device
> 
>       void *pattern[VIDEO_MAX_PLANES];
>       unsigned int patternsize[VIDEO_MAX_PLANES];
> +
> +     bool write_buffer_prefix;
>  };
> 
>  static bool video_is_mplane(struct device *dev)
> @@ -1546,13 +1548,22 @@ static void video_save_image(struct device *dev,
> struct v4l2_buffer *buf,
> 
>       for (i = 0; i < dev->num_planes; i++) {
>               unsigned int length;
> +             unsigned int data_offset = 0;
> 
> -             if (video_is_mplane(dev))
> +             if (video_is_mplane(dev)) {
>                       length = buf->m.planes[i].bytesused;
> -             else
> +                     data_offset = buf->m.planes[i].data_offset;
> +             } else {
>                       length = buf->bytesused;
> +             }
> +
> +             if (!dev->write_buffer_prefix)
> +                     length -= data_offset;
> +             else
> +                     data_offset = 0;

Nitpicking here, I find it a bit hard to read that you're adjusting the length 
here but still keep the offset to add it to the buffer address below. How 
about something like

                void *data = dev->buffers[buf->index].mem[i];
                unsigned int length;

                if (video_is_mplane(dev)) {
                        length = buf->m.planes[i].bytesused;

                        if (!dev->write_buffer_prefix) {
                                data += buf->m.planes[i].data_offset;
                                length -= buf->m.planes[i].data_offset;
                        }

                } else {
                        length = buf->bytesused;
                }

                ret = write(fd, data, length);

> -             ret = write(fd, dev->buffers[buf->index].mem[i], length);
> +             ret = write(fd, dev->buffers[buf->index].mem[i] + data_offset,
> +                         length);
>               if (ret < 0) {
>                       printf("write error: %s (%d)\n", strerror(errno), 
> errno);
>                       break;
> @@ -1717,6 +1728,7 @@ static void usage(const char *argv0)
>       printf("-t, --time-per-frame num/denom  Set the time per frame (eg. 
> 1/25 
=
> 25 fps)\n"); printf("-u, --userptr                    Use the user pointers 
> streaming
> method\n"); printf("-w, --set-control 'ctrl value'    Set control 'ctrl' to
> 'value'\n"); +        printf("    --buffer-prefix             Write portions 
> of buffer 
before
> data_offset\n"); printf("    --buffer-size            Buffer size in 
> bytes\n");
>       printf("    --enum-formats              Enumerate formats\n");
>       printf("    --enum-inputs               Enumerate inputs\n");
> @@ -1749,10 +1761,12 @@ static void usage(const char *argv0)
>  #define OPT_BUFFER_SIZE              268
>  #define OPT_PREMULTIPLIED    269
>  #define OPT_QUEUE_LATE               270
> +#define OPT_BUFFER_PREFIX    271
> 
>  static struct option opts[] = {
>       {"buffer-size", 1, 0, OPT_BUFFER_SIZE},
>       {"buffer-type", 1, 0, 'B'},
> +     {"buffer-prefix", 1, 0, OPT_BUFFER_PREFIX},
>       {"capture", 2, 0, 'c'},
>       {"check-overrun", 0, 0, 'C'},
>       {"delay", 1, 0, 'd'},
> @@ -2016,6 +2030,8 @@ int main(int argc, char *argv[])
>               case OPT_USERPTR_OFFSET:
>                       userptr_offset = atoi(optarg);
>                       break;
> +             case OPT_BUFFER_PREFIX:
> +                     dev.write_buffer_prefix = true;
>               default:
>                       printf("Invalid option -%c\n", c);
>                       printf("Run %s -h for help.\n", argv[0]);

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to