Re: [EXT] Re: [PATCH] media: amphion: only insert the first sequence startcode for vc1l format

[Hans Verkuil]

On 29/06/2022 14:53, Ming Qian wrote:
>> From: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
>> Sent: Wednesday, June 29, 2022 8:10 PM
>> To: Ming Qian <ming.qian@xxxxxxx>; mchehab@xxxxxxxxxx
>> Cc: shawnguo@xxxxxxxxxx; robh+dt@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx;
>> kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; dl-linux-imx <linux-
>> imx@xxxxxxx>; linux-media@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>> Subject: [EXT] Re: [PATCH] media: amphion: only insert the first sequence
>> startcode for vc1l format
>>
>> Caution: EXT Email
>>
>> On 28/06/2022 07:20, Ming Qian wrote:
>>> For some formats, the amphion vpu requires startcode before sequence
>>> and frame, such as vc1, vp8.
>>>
>>> But for V4L2_PIX_FMT_VC1_ANNEX_L, only the first sequence startcode is
>>> needed, the extra startcode will cause decoding error.
>>> So after seek, we don't need to insert the sequence startcode.
>>>
>>> In other words, for V4L2_PIX_FMT_VC1_ANNEX_L, the vpu doesn't
>> support
>>> dynamic resolution change.
>>
>> Shouldn't V4L2_FMT_FLAG_DYN_RESOLUTION be removed from that format
>> since it doesn't support this feature?
>>
>> Regards,
>>
>>         Hans
>>
> 
> I have a question, for format VC1L,  the V4L2_EVENT_SOURCE_CHANGE event still may be sent at the beginning,
> If the parameters parsed from the first sequence header are different from those previously established.
> 
> So should I remove the V4L2_FMT_FLAG_DYN_RESOLUTION flag or not?

Yes. That flag only applies if resolution changes can happen after the first
frame was decoded. It does not apply to the initial resolution change which
happens before decoding the first frame (i.e. after reading the header(s) at
the start).

Regards,

	Hans

> 
> Ming
> 
>>>
>>> Fixes: 145e936380edb ("media: amphion: implement malone decoder rpc
>>> interface")
>>> Signed-off-by: Ming Qian <ming.qian@xxxxxxx>
>>> ---
>>>  drivers/media/platform/amphion/vdec.c       | 1 +
>>>  drivers/media/platform/amphion/vpu.h        | 1 +
>>>  drivers/media/platform/amphion/vpu_malone.c | 2 ++
>>>  drivers/media/platform/amphion/vpu_rpc.h    | 7 ++++++-
>>>  4 files changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/platform/amphion/vdec.c
>>> b/drivers/media/platform/amphion/vdec.c
>>> index 09d4f27970ec..51218a41a5ac 100644
>>> --- a/drivers/media/platform/amphion/vdec.c
>>> +++ b/drivers/media/platform/amphion/vdec.c
>>> @@ -731,6 +731,7 @@ static void vdec_stop_done(struct vpu_inst *inst)
>>>       vdec->eos_received = 0;
>>>       vdec->is_source_changed = false;
>>>       vdec->source_change = 0;
>>> +     inst->total_input_count = 0;
>>>       vpu_inst_unlock(inst);
>>>  }
>>>
>>> diff --git a/drivers/media/platform/amphion/vpu.h
>>> b/drivers/media/platform/amphion/vpu.h
>>> index e56b96a7e5d3..f914de6ed81e 100644
>>> --- a/drivers/media/platform/amphion/vpu.h
>>> +++ b/drivers/media/platform/amphion/vpu.h
>>> @@ -258,6 +258,7 @@ struct vpu_inst {
>>>       struct vpu_format cap_format;
>>>       u32 min_buffer_cap;
>>>       u32 min_buffer_out;
>>> +     u32 total_input_count;
>>>
>>>       struct v4l2_rect crop;
>>>       u32 colorspace;
>>> diff --git a/drivers/media/platform/amphion/vpu_malone.c
>>> b/drivers/media/platform/amphion/vpu_malone.c
>>> index c62b49e85060..f4a488bf9880 100644
>>> --- a/drivers/media/platform/amphion/vpu_malone.c
>>> +++ b/drivers/media/platform/amphion/vpu_malone.c
>>> @@ -1314,6 +1314,8 @@ static int
>> vpu_malone_insert_scode_vc1_l_seq(struct malone_scode_t *scode)
>>>       int size = 0;
>>>       u8 rcv_seqhdr[MALONE_VC1_RCV_SEQ_HEADER_LEN];
>>>
>>> +     if (scode->inst->total_input_count)
>>> +             return 0;
>>>       scode->need_data = 0;
>>>
>>>       ret = vpu_malone_insert_scode_seq(scode,
>>> MALONE_CODEC_ID_VC1_SIMPLE, sizeof(rcv_seqhdr)); diff --git
>>> a/drivers/media/platform/amphion/vpu_rpc.h
>>> b/drivers/media/platform/amphion/vpu_rpc.h
>>> index 25119e5e807e..7eb6f01e6ab5 100644
>>> --- a/drivers/media/platform/amphion/vpu_rpc.h
>>> +++ b/drivers/media/platform/amphion/vpu_rpc.h
>>> @@ -312,11 +312,16 @@ static inline int vpu_iface_input_frame(struct
>> vpu_inst *inst,
>>>                                       struct vb2_buffer *vb)  {
>>>       struct vpu_iface_ops *ops = vpu_core_get_iface(inst->core);
>>> +     int ret;
>>>
>>>       if (!ops || !ops->input_frame)
>>>               return -EINVAL;
>>>
>>> -     return ops->input_frame(inst->core->iface, inst, vb);
>>> +     ret = ops->input_frame(inst->core->iface, inst, vb);
>>> +     if (ret < 0)
>>> +             return ret;
>>> +     inst->total_input_count++;
>>> +     return ret;
>>>  }
>>>
>>>  static inline int vpu_iface_config_memory_resource(struct vpu_inst *inst,
>