Re: [PATCH v3 2/2] media: amphion: Add a frame flush mode for decoder

["Ming Qian(OSS)" <ming.qian@oss.nxp.com>]

Hi Sebastian ,

On 2025/3/27 15:48, Sebastian Fricke wrote:
> Hey Ming,
> 
> On 05.03.2025 14:26, ming.qian@oss.nxp.com wrote:
>> From: Ming Qian <ming.qian@oss.nxp.com>
>>
>> By default the amphion decoder will pre-parse 3 frames before starting
>> to decode the first frame. Alternatively, a block of flush padding data
>> can be appended to the frame, which will ensure that the decoder can
>> start decoding immediately after parsing the flush padding data, thus
>> potentially reducing decoding latency.
>>
>> This mode was previously only enabled, when the display delay was set to
>> 0. Allow the user to manually toggle the use of that mode via a module
>> parameter called frame_flush_mode, which enables the mode without
>> changing the display order.
>>
>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>> ---
>> v3
>> - Improve commit message as recommended
>> - Add some comments to avoid code looks cryptic
>>
>> drivers/media/platform/amphion/vpu_malone.c | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/amphion/vpu_malone.c 
>> b/drivers/media/platform/amphion/vpu_malone.c
>> index 1d9e10d9bec1..4ef9810d8142 100644
>> --- a/drivers/media/platform/amphion/vpu_malone.c
>> +++ b/drivers/media/platform/amphion/vpu_malone.c
>> @@ -25,6 +25,10 @@
>> #include "vpu_imx8q.h"
>> #include "vpu_malone.h"
>>
>> +static bool frame_flush_mode;
>> +module_param(frame_flush_mode, bool, 0644);
>> +MODULE_PARM_DESC(frame_flush_mode, "Set low latency flush mode: 0 
>> (disable) or 1 (enable)");
>> +
>> #define CMD_SIZE            25600
>> #define MSG_SIZE            25600
>> #define CODEC_SIZE            0x1000
>> @@ -1579,7 +1583,15 @@ static int vpu_malone_input_frame_data(struct 
>> vpu_malone_str_buffer __iomem *str
>>
>>     vpu_malone_update_wptr(str_buf, wptr);
>>
>> -    if (disp_imm && !vpu_vb_is_codecconfig(vbuf)) {
>> +    /*
>> +     * Enable the low latency flush mode if display delay is set to 0
>> +     * or parameter frame_flush_mode is set to 1.
> 
> s/or parameter frame_flush_mode is set to 1./
>    or the frame flush mode if it is set to 1./
> 
I will apply your suggestion.

>> +     * The low latency flush mode requires some padding data to be 
>> appended after each frame,
> 
> s/appended after each/appended to each/
> (the word append implies that something is added after something else)
> 
Got it
>> +     * but don't put it in between the sequence header and frame.
> 
> s/but don't put it in between the sequence header and frame./
>    but there must not be any padding data between the sequence header 
> and the frame./
> 
> (As this is not a suggestion for the developer but a description of what
> the code does)
> 
Got it
>> +     * Only H264 and HEVC decoder support this module yet,
> 
> s/decoder/formats/
> 
> I'd rewrite this part:
> This module is currently only supported for the H264 and HEVC formats,
> 
> but that is only because this sounds more natural to me.
> 
I will apply your suggestion.

>> +     * for other formats, vpu_malone_add_scode() will return 0.
>> +     */
>> +    if ((disp_imm || frame_flush_mode) && 
>> !vpu_vb_is_codecconfig(vbuf)) {
>>         ret = vpu_malone_add_scode(inst->core->iface,
>>                        inst->id,
>>                        &inst->stream_buffer,
>> -- 
>> 2.43.0-rc1
>>
>>
> 
> Thank you!
> 
> Regards,
> Sebastian Fricke

Thank you very much for your help in expressing.

Regards,
Ming