2008/6/24 Laurent Pinchart <[EMAIL PROTECTED]>:
> A call to kmalloc with GFP_ATOMIC will probably faster than with GFP_KERNEL
> (although I'm not sure there are not some corner cases where the second
> could
> be faster than the first). This is however not the reason why you can't use
> GFP_KERNEL here: code running in interrupt context is not allowed to sleep,
> so must use GFP_ATOMIC. This is not a speed issue.
>
Thank you for the explaination, now it is more clear!! ;)
> pixel_size is misnamed as it stores the size of a whole length of video
> data.
>
You are right...I'm going to change "pixel_size" into "line_size"!!
> > + to_be_copied = 0;
> > + while( nbytes > 0 ) { // Are there other bytes to be copied?
>
> Please respect the kernel coding style (Documentation/CodingStyle). Don't
> put
> spaces inside parentheses, add a space between 'while' and the opening
> parenthesis, don't put any statement on the same line as the 'else' keyword
> and keep you code 80 characters wide.
>
Really really thank you for informing me about that!! As you know it is the
first time I work on kernel, modules and driver!! ;)
> > + shift_right = buf->buf.bytesused % pixel_size;
> > + if ( nbytes > pixel_size - shift_right)
> > + to_be_copied = pixel_size - shift_right ;
> > + else to_be_copied = nbytes;
> > +
> > + if (shift_right > 0 )
> > + buf_offset = to_be_copied - shift_right ;
> > + else buf_offset = pixel_size;
> > + memcpy( queue->mem + buf->buf.m.offset + pixel_size *
> > video->streaming->cur_frame->wHeight - buf->buf.bytesused - buf_offset,
> > + data,
> > + to_be_copied ) ;
>
> If I'm not mistaken this will only work when the data packets contain
> entire
> lines of image data. This is not guaranteed.
>
No you are wrong, I've considered that case. ;) The algorithm works fine
because I've already tested it!!
But also because I've just tested your driver with luvcview and different
solution and I found out that:
- the function above is called about 1040 times for a 1280x1024 resolution;
- each time "len" is not the same and it's not sure len is a multiple of the
size of an entire line of image;
- "data" stores information about a sequence of pixel, and quite never this
sequence starts from a pixel on the left edge of the image and ends on a
pixel on the right edge.
So, when I was working on the algorithm I had in mind all this stuff!! ;)
> To reverse the image you
> shouldn't use memcpy but instead roll your own memcpy version that will
> decrement the destination pointer instead of incrementing it. It shouldn't
> be
> more complex than that. Just fill the video buffer backwards starting at
> the
> end instead of forward starting at the beginning.
>
Why should I do that? What will change? The algorithm already start to fill
the "mem" buffer starting from the end of the image memory area.
My algorithm just reverse the image, but gives back a mirrored one because
it reverse the order of image lines.
However I also made a new version that gives back not mirrored images (
http://ubuntuforums.org/showthread.php?t=838210)
> > +
> > + data += to_be_copied;
> > + buf->buf.bytesused += to_be_copied ;
>
> In the end all of nbytes will have been copied, so you can increment
> buf->buf.bytesused in one go outside the loop.
>
Yes I know, but I increment byteused each loop because this value is used in
the memcpy()
> > + if (len >= maxlen) {
>
> Don't modify this (see my previous e-mail).
>
Ok, thanks for the suggestion!! ;)
> > Unluckly also if now I don't allocate any memory, with bigger resolutions
> > the final video seems to be a little bit slower than with the allocation
> > (previous solution).
>
> Can you benchmark that ? I haven't
>
Have you any ideas about how I can do a benchmark?
Thank you for your help!!! ;)
Regards
Marco Argiolas
_______________________________________________
Linux-uvc-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
