On Sat, 2019-11-09 at 11:40 +0100, Hans Verkuil wrote: > On 10/4/19 8:07 AM, Tomasz Figa wrote: > > Hi Ezequiel, > > > > On Fri, Oct 4, 2019 at 4:09 AM Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx> wrote: > > > Hi all, > > > > > > The Hantro G1 VPU post-processor block can be pipelined with > > > the decoder hardware, allowing to perform operations such as > > > color conversion, scaling, rotation, cropping, among others. > > > > > > When the post-processor is enabled, the decoder hardware > > > gets its own set of NV12 buffers (the native decoder format), > > > and the post-processor is the owner of the CAPTURE buffers. > > > > > > This allows the application get processed > > > (scaled, converted, etc) buffers, completely transparently. > > > > > > This feature is implemented by exposing the post-processed pixel > > > formats on ENUM_FMT. When the application sets a pixel format > > > other than NV12, and if the post-processor is MC-linked, > > > the driver will make use of it, transparently. > > > > > > On this v2, the media controller API is now required > > > to "enable" (aka link, in topology jargon) the post-processor. > > > Therefore, applications now have to explicitly request this feature. > > > > > > For instance, the post-processor is enabled using the media-ctl > > > tool: > > > > > > media-ctl -l "'decoder':1 -> 'rockchip,rk3288-vpu-dec':0[0]" > > > media-ctl -l "'postproc':1 -> 'rockchip,rk3288-vpu-dec':0[1]" > > > > > > v4l2-ctl -d 1 --list-formats > > > ioctl: VIDIOC_ENUM_FMT > > > Type: Video Capture Multiplanar > > > > > > [0]: 'NV12' (Y/CbCr 4:2:0) > > > [1]: 'YUYV' (YUYV 4:2:2) > > > > > > Patches 1 and 2 are simply cleanups needed to easier integrate the > > > post-processor. Patch 2 is a bit invasive, but it's a step > > > forward towards merging the implementation for RK3399 and RK3288. > > > > > > Patch 3 re-works the media-topology, making the decoder > > > a v4l2_subdevice, instead of a bare entity. This allows > > > to introduce a more accurate of the decoder+post-processor complex. > > > > > > Patch 4 introduces the post-processing support. > > > > > > This is tested on RK3288 platforms with MPEG-2, VP8 and > > > H264 streams, decoding to YUY2 surfaces. For now, this series > > > is only adding support for NV12-to-YUY2 conversion. > > > > > > The design and implementation is quite different from v1: > > > > > > * The decoder->post-processor topology is now exposed > > > explicitly and applications need to configure the pipeline. > > > By default, the decoder is enabled and the post-processor > > > is disabled. > > > > > > > First of all, thanks for working on this. While from Chromium point of > > view there isn't any practical use case for this feature, it could > > possibly be valuable for some other platforms. > > > > I still see a problem with the current design. Mem-to-mem decoders are > > commonly used in a multi-instance fashion, but media topology is > > global. That means that when having two applications on the system > > decoding their own videos, one might be stepping on the other by > > changing the topology. > > > > I believe that if we want this to be really usable, we would need to > > make the media topology per instance, but that's a significant change > > to the media subsystem. Maybe we could pursue the other options I > > suggested in the previous revision instead, like ordering the formats > > returned by enum_fmt by native first and adding format flags that > > would tell the userspace that the format can have some performance > > and/or power penalty? > > I discussed this with Ezequiel in Lyon, and my preference is also to > not rely on the media controller, but instead order the formats with > the native formats first, followed by the formats that need this additional > processing step. A format flag can be added, but I feel that it is better > to wait with that until it is clear that there is a real need for it. > > It would be good to document in the ENUM_FMT ioctl that formats that > require additional processing are at the end of the format list. > > Ezequiel, I'm marking this series as "Changes Requested" in patchwork. > Thanks! I should revisit this and post a new series soon. Ezequiel
