Re: [RFCv3 PATCH 00/33] Core and vb2 enhancements

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans

On Thu, 28 Jun 2012, Hans Verkuil wrote:

> Hi all,
> 
> This is the third version of this patch series.
> 
> The first version is here:
> 
> http://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg47558.html

Nice to see an owner concept added to the vb2. In soc-camera we're also 
using a concept of a "streamer" user. This is the user, that first calls 
one of data-flow related ioctl()s, like s_fmt(), streamon(), streamoff() 
and all buffer queue-related operations. I realise that this your 
patch-set only deals with the buffer queue, but in principle, do you think 
it would make sense to use such a concept globally? We probably don't want 
to let other processes mess with any of the above calls as long as one 
process is actively managing the streaming.

Thanks
Guennadi

> Changes since RFCv2:
> 
> - Rebased to staging/for_v3.6.
> 
> - Incorporated Laurent's review comments in patch 22: vb2-core: refactor reqbufs/create_bufs.
> 
> Changes since RFCv1:
> 
> - Incorporated all review comments from Hans de Goede and Laurent Pinchart (Thanks!)
>   except for splitting off the vb2 helper functions into a separate source. I decided
>   to keep it together with the vb2-core code.
> 
> - Improved commit messages, added more comments to the code.
> 
> - The owner filehandle and the queue lock are both moved to struct vb2_queue since
>   these are a property of the queue.
> 
> - The debug function has a new 'write_only' boolean: some debug functions can only
>   print a subset of the arguments if it is called by an _IOW ioctl. The previous
>   patch series split this up into two functions. Handling the debug function for
>   a write-only ioctl is annoying at the moment: you have to print the arguments
>   before calling the ioctl since the ioctl can overwrite arguments. I am considering
>   changing the op argument to const for such ioctls and see if any driver is
>   actually messing around with the contents of such structs. If we can guarantee
>   that drivers do not change the argument struct, then we can simplify the debug
>   code.
> 
> - All debugging is now KERN_DEBUG instead of KERN_INFO.
> 
> I still have one outstanding question: should anyone be able to call mmap() or
> only the owner of the vb2 queue? Right now anyone can call mmap().
> 
> Comments are welcome, but if I don't see any in the next 2-3 days, then I'll make
> a pull request for this on Sunday.
> 
> Regards,
> 
>         Hans
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux