On 5/21/19 11:09 AM, Tomasz Figa wrote: > Hi Stan, > > On Mon, May 20, 2019 at 11:47 PM Stanimir Varbanov > <stanimir.varbanov@xxxxxxxxxx> wrote: >> >> Hi Tomasz, >> >> On 4/24/19 3:39 PM, Tomasz Figa wrote: >>> On Wed, Apr 24, 2019 at 9:15 PM Stanimir Varbanov >>> <stanimir.varbanov@xxxxxxxxxx> wrote: >>>> >>>> Hi Hans, >>>> >>>> On 2/15/19 3:44 PM, Hans Verkuil wrote: >>>>> Hi Stanimir, >>>>> >>>>> I never paid much attention to this patch series since others were busy >>>>> discussing it and I had a lot of other things on my plate, but then I heard >>>>> that this patch made G_FMT blocking. >>>> >>>> OK, another option could be to block REQBUF(CAPTURE) until event from hw >>>> is received that the stream is parsed and the resolution is correctly >>>> set by application. Just to note that I'd think to this like a temporal >>>> solution until gstreamer implements v4l events. >>>> >>>> Is that looks good to you? >>> >>> Hmm, I thought we concluded that gstreamer sets the width and height >>> in OUTPUT queue before querying the CAPTURE queue and so making the >>> driver calculate the CAPTURE format based on what's set on OUTPUT >>> would work fine. Did I miss something? >> >> Nobody is miss something. >> >> First some background about how Venus implements stateful codec API. >> >> The Venus firmware can generate two events "sufficient" and >> "insufficient" buffer requirements (this includes decoder output buffer >> size and internal/scratch buffer sizes). Presently I always set minimum >> possible decoder resolution no matter what the user said, and by that >> way I'm sure that "insufficient" event will always be triggered by the >> firmware (the other reason to take this path is because this is the >> least-common-divider for all supported Venus hw/fw versions thus common >> code in the driver). The reconfiguration (during codec Initialization >> sequence) is made from STREAMON(CAPTURE) context. Now, to make that >> re-configuration happen I need to wait for "insufficient" event from >> firmware in order to know the real coded resolution. >> >> In the case of gstreamer where v4l2_events support is missing I have to >> block (wait for firmware event) REQBUF(CAPTURE) (vb2::queue_setup) or >> STREAMON(CAPTURE) (vb2::start_streaming). >> >> I tried to set the coded resolution to the firmware as-is it set by >> gstreamer but then I cannot receive the "sufficient" event for VP8 and >> VP9 codecs. So I return back to the solution with minimum resolution above. >> >> I'm open for suggestions. > > I think you could still keep setting the minimum size and wait for the > "insufficient" event. At the same time, you could speculatively > advertise the expected "sufficient" size on the CAPTURE queue before > the hardware signals those. Even if you mispredict them, you'll get > the event, update the CAPTURE resolution and send the source change > event to the application, which would then give you the correct > buffers. Would that work for you? As I understand it this still would require event support, which gstreamer doesn't have. I think it is OK to have REQBUFS sleep in this case. However, I would only enable this behavior if the application didn't subscribe to the SOURCE_CHANGE event. That's easy enough to check in the driver. And that means that if the application is well written, then the driver will behave in a completely standard way that the compliance test can check. Regards, Hans
