[libcamera-devel] Video recording on PinePhone Re: libcamera on pinephone

[Laurent Pinchart]

On Fri, Jul 15, 2022 at 12:20:13AM +0200, Pavel Machek wrote:
> Hi!
> 
> > > Wow. Ok, uncached explains things. Is it easy to switch to do read()
> > > from /dev/videoX? That should be easy to do, not require ABI changes,
> > > and should improve performance in this case...
> > 
> > read() is deprecated and won't help much as there will still be a copy
> > reading from uncached memory. You can get the same result with a
> > memcpy() of the whole frame in the application.
> 
> First, let me begin by saying that passing uncached memory to userland
> is rude and maybe dangerous. There are some CPU errata, for
> example. Placing futex in uncached memory might be fun. Userspace is
> not really equipped to deal with uncached memory.

I'd be curious to know use cases for placing a futex in a video buffer
though :-)

> Second, no. If we do read(), it is simple interface, kernel will be
> doing the copying, and it is free to do the usual magic to speed
> things up. (I suppose simply working with memory as cached and using
> flush_dcache_range). 

Regardless of whether or not drivers could speed things up with read(),
read() is not the way forward. It's deprecated for V4L2 camera devices,
so we won't use it in libcamera, the way forward is to fix V4L2 memory
mapping with cache management.

> With current interface, I get a ~18fps with 1920x1080 images.
> 
> With read/write, I get 82fps when doing file-to-file copy (with
> additional open/close syscalls that should not really be needed) and
> 145 fps when copying from /dev/null.
> 
> You mentioned that new interface for passing cacheable buffers would
> be needed. That sounds like a lot of work but I'll gladly help with
> testing.

Yes, I think there would be quite some work required there. I would also
be happy to test it once someone will do the work :-)

> If we start using read(), we will get speedup over current situation
> -- I should be able to get 30fps on the pinephone -- and advantage is
> that it is existing interface. All we need to do is to remove the
> "deprecated" tag and implement it for our hardware.

I doubt that would be accepted in the mainline kernel.

> Do you see any other short-term solutions?

No, I don't think there's any short-term solution. Someone needs to do
the work on the kernel side.

> PS: Benchmark attached. It is 190MB/sec on the pinephone; I kind of
> expected more, but.. it crosses kernel-user boundary and we really
> only need 60MB/sec for fully using all camera capabilities.
> 
> #include <stdio.h>
> #include <fcntl.h>
> 
> /* cat /dev/zero | head -c 2073600 > /tmp/delme.0 
>  
> mobian at mobian:~/g/tui/pp$ time ./syscalltest 
> 0.00user 1.19system 1.55 (0m1.556s) elapsed 76.82%CPU 
> echo $? 
>  
> file-to-file: 
> 100 frames in 1.219 seconds -> 82fps. 
> zero-to-file: 
> in 0m0.690s -> 145fps. 
>  */
> 
> #define	ZERO
> #define SIZE 2073600
> 
> int main(void)
> {
>   int f1, f2;
>   int i;
>   static char buf[SIZE];
> 
> #ifdef ZERO
>   f1 = open("/dev/zero", O_RDWR);
> #endif
>   f2 = open("/tmp/delme.1", O_CREAT | O_RDWR, 0777);
> 
>   for (i=0; i<100; i++) {
> #ifndef ZERO
>     f1 = open("/tmp/delme.0", O_RDWR);
> #endif
>     if (SIZE != read(f1, buf, SIZE))
>       return 1;
> #ifndef ZERO
>     close(f1);
> #endif
>     if (SIZE != write(f2, buf, SIZE))
>       return 2;
>   }
>   return 0;
> }

-- 
Regards,

Laurent Pinchart