Hi, On Wed, 2018-08-22 at 14:33 -0300, Ezequiel Garcia wrote: > On Wed, 2018-08-22 at 16:10 +0200, Paul Kocialkowski wrote: > > Hi, > > > > On Tue, 2018-08-21 at 17:52 +0900, Tomasz Figa wrote: > > > Hi Hans, Paul, > > > > > > On Mon, Aug 6, 2018 at 6:29 PM Paul Kocialkowski > > > <paul.kocialkowski@xxxxxxxxxxx> wrote: > > > > > > > > On Mon, 2018-08-06 at 11:23 +0200, Hans Verkuil wrote: > > > > > On 08/06/2018 11:13 AM, Paul Kocialkowski wrote: > > > > > > Hi, > > > > > > > > > > > > On Mon, 2018-08-06 at 10:32 +0200, Hans Verkuil wrote: > > > > > > > On 08/06/2018 10:16 AM, Paul Kocialkowski wrote: > > > > > > > > On Sat, 2018-08-04 at 15:50 +0200, Hans Verkuil wrote: > > > > > > > > > Regarding point 3: I think this should be documented next to the pixel format. I.e. > > > > > > > > > the MPEG-2 Slice format used by the stateless cedrus codec requires the request API > > > > > > > > > and that two MPEG-2 controls (slice params and quantization matrices) must be present > > > > > > > > > in each request. > > > > > > > > > > > > > > > > > > I am not sure a control flag (e.g. V4L2_CTRL_FLAG_REQUIRED_IN_REQ) is needed here. > > > > > > > > > It's really implied by the fact that you use a stateless codec. It doesn't help > > > > > > > > > generic applications like v4l2-ctl or qv4l2 either since in order to support > > > > > > > > > stateless codecs they will have to know about the details of these controls anyway. > > > > > > > > > > > > > > > > > > So I am inclined to say that it is not necessary to expose this information in > > > > > > > > > the API, but it has to be documented together with the pixel format documentation. > > > > > > > > > > > > > > > > I think this is affected by considerations about codec profile/level > > > > > > > > support. More specifically, some controls will only be required for > > > > > > > > supporting advanced codec profiles/levels, so they can only be > > > > > > > > explicitly marked with appropriate flags by the driver when the target > > > > > > > > profile/level is known. And I don't think it would be sane for userspace > > > > > > > > to explicitly set what profile/level it's aiming at. As a result, I > > > > > > > > don't think we can explicitly mark controls as required or optional. > > > > > > I'm not sure this is entirely true. The hardware may need to be > > > explicitly told what profile the video is. It may even not be the > > > hardware, but the driver itself too, given that the profile may imply > > > the CAPTURE pixel format, e.g. for VP9 profiles: > > > > > > profile 0 > > > color depth: 8 bit/sample, chroma subsampling: 4:2:0 > > > profile 1 > > > color depth: 8 bit, chroma subsampling: 4:2:0, 4:2:2, 4:4:4 > > > profile 2 > > > color depth: 10–12 bit, chroma subsampling: 4:2:0 > > > profile 3 > > > color depth: 10–12 bit, chroma subsampling: 4:2:0, 4:2:2, 4:4:4 > > > > > > (reference: https://en.wikipedia.org/wiki/VP9#Profiles) > > > > I think it would be fair to expect userspace to select the right > > destination format (and maybe have the driver error if there's a > > mismatch with the meta-data) instead of having the driver somewhat > > expose what format should be used. > > > > But maybe this would be an API violation, since all the enumerated > > formats are probably supposed to be selectable? > > > > We could also look at it the other way round and consider that selecting > > an exposed format is always legit, but that it implies passing a > > bitstream that matches it or the driver will error (because of an > > invalid bitstream passed, not because of a "wrong" selected format). > > > > The API requires the user to negotiate via TRY_FMT/S_FMT. The driver > usually does not return error on invalid formats, and simply return > a format it can work with. I think the kernel-user contract has to > guarantee if the driver accepted a given format, it won't fail to > encoder or decode. Well, the issue here is that in order to correctly enumerate the formats, the driver needs to be aware of: 1. in what destination format the bitstream data is decoded to; 2. what format convesions the VPU can do. Step 1 is known by userspace but is only passed to the driver with the bitstream metadata from controls. This is much too late for trimming the list of supported formats. I'm not sure that passing an extra information to the driver early to trim the list would make sense, because it comes to down to telling the driver what format to target and then asking the driver was format it supports, which is rather redundant. I think the information we need to expose to userspace is whether the driver supports a destination format that does not match the bitstream format. We could make it part of the spec that, by lack of this indication, the provided bitstream is expected to match the format that was selected. > I think that's why it makes sense for stateless drivers to set the > profile/levels for a given job, in order to properly negotiate > the input and output formats (for both encoders and decoders). I still fail to follow this logic. As far as I understood, profile/level are indications of hardware capabilities required to decode the video, not an indication of the precise features selected for decoding, especially when it comes the format. As Tomasz mentionned in the previous email, various formats may be supported by the same profile, so setting the profile/level does not indicate which format is appropriate for decoding the bitstream. > [1] https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-g-fmt.html -- Paul Kocialkowski, Bootlin (formerly Free Electrons) Embedded Linux and kernel engineering https://bootlin.com
Attachment:
signature.asc
Description: This is a digitally signed message part
