From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Subject: Re: [PATCH 00/10] Venus stateful Codec API Date: Mon, 28 Jan 2019 13:30:32 +0900 Message-ID: References: <20190117162008.25217-1-stanimir.varbanov@linaro.org> <10deb3d1-2b10-43fe-bc77-4465f561c90a@linaro.org> <80b97fd8-bd3a-df74-c611-5da11bd7adc6@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Alexandre Courbot Cc: Stanimir Varbanov , Linux Media Mailing List , Mauro Carvalho Chehab , Hans Verkuil , LKML , linux-arm-msm , Vikash Garodia , Malathi Gottam List-Id: linux-arm-msm@vger.kernel.org On Mon, Jan 28, 2019 at 1:16 PM Alexandre Courbot wrote: > > On Fri, Jan 25, 2019 at 7:35 PM Stanimir Varbanov > wrote: > > > > Hi Alex, > > > > On 1/25/19 7:34 AM, Alexandre Courbot wrote: > > > On Thu, Jan 24, 2019 at 7:13 PM Stanimir Varbanov > > > wrote: > > >> > > >> Hi Alex, > > >> > > >> Thank you for review and valuable comments! > > >> > > >> On 1/24/19 10:43 AM, Alexandre Courbot wrote: > > >>> Hi Stanimir, > > >>> > > >>> On Fri, Jan 18, 2019 at 1:20 AM Stanimir Varbanov > > >>> wrote: > > >>>> > > >>>> Hello, > > >>>> > > >>>> This aims to make Venus decoder compliant with stateful Codec API [1]. > > >>>> The patches 1-9 are preparation for the cherry on the cake patch 10 > > >>>> which implements the decoder state machine similar to the one in the > > >>>> stateful codec API documentation. > > >>> > > >>> Thanks *a lot* for this series! I am still stress-testing it against > > >>> the Chromium decoder tests, but so far it has been rock-solid. I have > > >>> a few inline comments on some patches ; I will also want to review the > > >>> state machine more thoroughly after refreshing my mind on Tomasz doc, > > >>> but this looks pretty promising already. > > >> > > >> I'm expecting problems with ResetAfterFirstConfigInfo. I don't know why > > >> but this test case is very dirty. I'd appreciate any help to decipher > > >> what is the sequence of v4l2 calls made by this unittest case. > > > > > > I did not see any issue with ResetAfterFirstConfigInfo, however > > > ResourceExhaustion seems to hang once in a while. But I could already > > > see this behavior with the older patchset. > > > > Is it hangs badly? > > It just stops processing, no crash. I need to investigate more closely. > > > > > > > In any case I plan to thoroughly review the state machine. I agree it > > > is a bit complex to grasp. > > > > yes the state machine isn't simple and I blamed Tomasz many times for > > that :) Don't shoot the messenger! I'm just carrying what hardware and userspace made for us. ;) > > If I feel inspired I may try to extract the useful part of your patch > into a framework that we can use everywhere. :) We don't want every > driver to reimplement this complex state machine and introduce new > bugs every time. Yeah, we definitely need such a framework, especially given how the m2m framework is overused for handling codecs. Best regards, Tomasz