Message-ID: <3d0315fa-14ca-dc34-81ae-467d9ed5133d@quicinc.com>
Date:   Tue, 7 Mar 2023 10:15:45 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH] media: venus: dec: Fix capture formats enumeration order
Content-Language: en-US
To:     Javier Martinez Canillas <javierm@redhat.com>,
        Jordan Crouse <jorcrous@amazon.com>
CC:     <linux-kernel@vger.kernel.org>, Albert Esteve <aesteve@redhat.com>,
        Stanimir Varbanov <stanimir.varbanov@linaro.org>,
        Sergio Lopez <slp@redhat.com>,
        Enric Balletbo i Serra <eballetb@redhat.com>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Stanimir Varbanov <stanimir.k.varbanov@gmail.com>,
        Vikash Garodia <quic_vgarodia@quicinc.com>,
        <linux-arm-msm@vger.kernel.org>, <linux-media@vger.kernel.org>
References: <20230210081835.2054482-1-javierm@redhat.com>
 <20230303220918.qr5ydbin3nye3qtz@amazon.com>
 <87h6uydwel.fsf@minerva.mail-host-address-is-not-set>
From:   Dikshita Agarwal <quic_dikshita@quicinc.com>
In-Reply-To: <87h6uydwel.fsf@minerva.mail-host-address-is-not-set>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 3/6/2023 3:38 PM, Javier Martinez Canillas wrote:
> Jordan Crouse <jorcrous@amazon.com> writes:
>
> Hello Jordan,
>
>> On Fri, Feb 10, 2023 at 09:18:35AM +0100, Javier Martinez Canillas wrote:
>>> Commit 9593126dae3e ("media: venus: Add a handling of QC08C compressed
>>> format") and commit cef92b14e653 ("media: venus: Add a handling of QC10C
>>> compressed format") added support for the QC08C and QC10C compressed
>>> formats respectively.
>>>
>>> But these also caused a regression, because the new formats where added
>>> at the beginning of the vdec_formats[] array and the vdec_inst_init()
>>> function sets the default format output and capture using fixed indexes
>>> of that array:
>>>
>>> static void vdec_inst_init(struct venus_inst *inst)
>>> {
>>> ...
>>> 	inst->fmt_out = &vdec_formats[8];
>>> 	inst->fmt_cap = &vdec_formats[0];
>>> ...
>>> }
>>>
>>> Since now V4L2_PIX_FMT_NV12 is not the first entry in the array anymore,
>>> the default capture format is not set to that as it was done before.
>>>
>>> Both commits changed the first index to keep inst->fmt_out default format
>>> set to V4L2_PIX_FMT_H264, but did not update the latter to keep .fmt_out
>>> default format set to V4L2_PIX_FMT_NV12.
>>>
>>> Rather than updating the index to the current V4L2_PIX_FMT_NV12 position,
>>> let's reorder the entries so that this format is the first entry again.
>>>
>>> This would also make VIDIOC_ENUM_FMT report the V4L2_PIX_FMT_NV12 format
>>> with an index 0 as it did before the QC08C and QC10C formats were added.
>>>
>>> Fixes: 9593126dae3e ("media: venus: Add a handling of QC08C compressed format")
>>> Fixes: cef92b14e653 ("media: venus: Add a handling of QC10C compressed format")
>>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> I just came across this issue independently and can confirm this patch fixes
>> the GStreamer V4L2 decoder on QRB5165.
>>
>> Tested-by: Jordan Crouse <jorcrous@amazon.com>
>>
> Thanks for testing it!
>
> Stanimir, can we please get this for v6.3 as well?

Hi Javier, Jordan

Could you please explain what regression/issue you see with patch?

venus hardware supports QC08C which provides better performance hence 
driver is publishing it as preferred color format.

if client doesn't support this or want to use any other format, they can 
set the desired format with s_fmt.

Thanks,

Dikshita