On Thu, Jan 31, 2019 at 12:06 AM Nicolas Dufresne <nicolas@xxxxxxxxxxxx> wrote: > > Le vendredi 25 janvier 2019 à 12:59 +0900, Tomasz Figa a écrit : > > On Fri, Jan 25, 2019 at 5:14 AM Nicolas Dufresne <nicolas@xxxxxxxxxxxx> wrote: > > > Le mercredi 23 janvier 2019 à 14:04 +0100, Hans Verkuil a écrit : > > > > > > Does this return the same set of formats as in the 'Querying Capabilities' phase? > > > > > > > > > > > > > > > > It's actually an interesting question. At this point we wouldn't have > > > > > the OUTPUT resolution set yet, so that would be the same set as in the > > > > > initial query. If we set the resolution (with some arbitrary > > > > > pixelformat), it may become a subset... > > > > > > > > But doesn't setting the capture format also set the resolution? > > > > > > > > To quote from the text above: > > > > > > > > "The encoder will derive a new ``OUTPUT`` format from the ``CAPTURE`` format > > > > being set, including resolution, colorimetry parameters, etc." > > > > > > > > So you set the capture format with a resolution (you know that), then > > > > ENUM_FMT will return the subset for that codec and resolution. > > > > > > > > But see also the comment at the end of this email. > > > > > > I'm thinking that the fact that there is no "unset" value for pixel > > > format creates a certain ambiguity. Maybe we could create a new pixel > > > format, and all CODEC driver could have that set by default ? Then we > > > can just fail STREAMON if that format is set. > > > > The state on the CAPTURE queue is actually not "unset". The queue is > > simply not ready (yet) and any operations on it will error out. > > My point was that it's just awkward to have this "not ready" state, in > which you cannot go back. And in which the enum-format will ignore the > format configured on the other side. > > What I wanted to say is that this special case is not really needed. > Yeah, I think we may actually end up going in that direction, as you probably noticed in the discussion over the "venus: dec: make decoder compliant with stateful codec API" patch [1]. [1] https://patchwork.kernel.org/patch/10768539/#22462703 > > > > Once the application sets the coded resolution on the OUTPUT queue or > > the decoder parses the stream information, the CAPTURE queue becomes > > ready and one can do the ioctls on it. > > > > > That being said, in GStreamer, I have split each elements per CODEC, > > > and now only enumerate the information "per-codec". That makes me think > > > this "global" enumeration was just a miss-use of the API / me learning > > > to use it. Not having to implement this rather complex thing in the > > > driver would be nice. Notably, the new Amlogic driver does not have > > > this "Querying Capabilities" phase, and with latest GStreamer works > > > just fine. > > > > What do you mean by "doesn't have"? Does it lack an implementation of > > VIDIOC_ENUM_FMT and VIDIOC_ENUM_FRAMESIZES? > > What it does is that it sets a default value for the codec format, so > if you just open the device and do enum_fmt/framesizes, you get that is > possible for the default codec that was selected. And I thin it's > entirely correct, doing ENUM_FMT(capture) without doing an > S_FMT(output) can easily be documented as undefined behaviour. Okay. > > For proper enumeration would be: > > for formats on OUTPUT device: > S_FMT(OUTPUT): > for formats on CAPTURE device: > ... > > (the pseudo for look represent an enum operation) And that's how it's defined in v3. There is no default state without any codec selected. Best regards, Tomasz
