* [PATCH] media: amphion: only insert the first sequence startcode for vc1l format
@ 2022-06-28 5:20 Ming Qian
2022-06-29 12:10 ` Hans Verkuil
2022-07-04 16:05 ` Nicolas Dufresne
0 siblings, 2 replies; 11+ messages in thread
From: Ming Qian @ 2022-06-28 5:20 UTC (permalink / raw)
To: mchehab, hverkuil-cisco
Cc: shawnguo, robh+dt, s.hauer, kernel, festevam, linux-imx,
linux-media, linux-kernel, linux-arm-kernel
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.
Fixes: 145e936380edb ("media: amphion: implement malone decoder rpc interface")
Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
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,
--
2.36.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] media: amphion: only insert the first sequence startcode for vc1l format
2022-06-28 5:20 [PATCH] media: amphion: only insert the first sequence startcode for vc1l format Ming Qian
@ 2022-06-29 12:10 ` Hans Verkuil
2022-06-29 12:53 ` [EXT] " Ming Qian
2022-07-04 16:05 ` Nicolas Dufresne
1 sibling, 1 reply; 11+ messages in thread
From: Hans Verkuil @ 2022-06-29 12:10 UTC (permalink / raw)
To: Ming Qian, mchehab
Cc: shawnguo, robh+dt, s.hauer, kernel, festevam, linux-imx,
linux-media, linux-kernel, linux-arm-kernel
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
>
> Fixes: 145e936380edb ("media: amphion: implement malone decoder rpc interface")
> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> ---
> 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,
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [EXT] Re: [PATCH] media: amphion: only insert the first sequence startcode for vc1l format
2022-06-29 12:10 ` Hans Verkuil
@ 2022-06-29 12:53 ` Ming Qian
2022-06-29 13:03 ` Hans Verkuil
0 siblings, 1 reply; 11+ messages in thread
From: Ming Qian @ 2022-06-29 12:53 UTC (permalink / raw)
To: Hans Verkuil, mchehab
Cc: shawnguo, robh+dt, s.hauer, kernel, festevam, dl-linux-imx,
linux-media, linux-kernel, linux-arm-kernel
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Sent: Wednesday, June 29, 2022 8:10 PM
> To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org
> Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux-
> imx@nxp.com>; linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org
> 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?
Ming
> >
> > Fixes: 145e936380edb ("media: amphion: implement malone decoder rpc
> > interface")
> > Signed-off-by: Ming Qian <ming.qian@nxp.com>
> > ---
> > 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,
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [EXT] Re: [PATCH] media: amphion: only insert the first sequence startcode for vc1l format
2022-06-29 12:53 ` [EXT] " Ming Qian
@ 2022-06-29 13:03 ` Hans Verkuil
2022-06-29 13:08 ` Ming Qian
0 siblings, 1 reply; 11+ messages in thread
From: Hans Verkuil @ 2022-06-29 13:03 UTC (permalink / raw)
To: Ming Qian, mchehab
Cc: shawnguo, robh+dt, s.hauer, kernel, festevam, dl-linux-imx,
linux-media, linux-kernel, linux-arm-kernel
On 29/06/2022 14:53, Ming Qian wrote:
>> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> Sent: Wednesday, June 29, 2022 8:10 PM
>> To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org
>> Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
>> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux-
>> imx@nxp.com>; linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
>> linux-arm-kernel@lists.infradead.org
>> 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@nxp.com>
>>> ---
>>> 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,
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [EXT] Re: [PATCH] media: amphion: only insert the first sequence startcode for vc1l format
2022-06-29 13:03 ` Hans Verkuil
@ 2022-06-29 13:08 ` Ming Qian
0 siblings, 0 replies; 11+ messages in thread
From: Ming Qian @ 2022-06-29 13:08 UTC (permalink / raw)
To: Hans Verkuil, mchehab
Cc: shawnguo, robh+dt, s.hauer, kernel, festevam, dl-linux-imx,
linux-media, linux-kernel, linux-arm-kernel
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Sent: Wednesday, June 29, 2022 9:03 PM
> To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org
> Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux-
> imx@nxp.com>; linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [EXT] Re: [PATCH] media: amphion: only insert the first
> sequence startcode for vc1l format
>
> Caution: EXT Email
>
> On 29/06/2022 14:53, Ming Qian wrote:
> >> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> >> Sent: Wednesday, June 29, 2022 8:10 PM
> >> To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org
> >> Cc: shawnguo@kernel.org; robh+dt@kernel.org;
> s.hauer@pengutronix.de;
> >> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux-
> >> imx@nxp.com>; linux-media@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> >> 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).
Got it, I'll remove it in V2 patch
>
> Regards,
>
> Hans
>
> >
> > Ming
> >
> >>>
> >>> Fixes: 145e936380edb ("media: amphion: implement malone decoder
> rpc
> >>> interface")
> >>> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> >>> ---
> >>> 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,
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] media: amphion: only insert the first sequence startcode for vc1l format
2022-06-28 5:20 [PATCH] media: amphion: only insert the first sequence startcode for vc1l format Ming Qian
2022-06-29 12:10 ` Hans Verkuil
@ 2022-07-04 16:05 ` Nicolas Dufresne
2022-07-05 2:00 ` [EXT] " Ming Qian
1 sibling, 1 reply; 11+ messages in thread
From: Nicolas Dufresne @ 2022-07-04 16:05 UTC (permalink / raw)
To: Ming Qian, mchehab, hverkuil-cisco
Cc: shawnguo, robh+dt, s.hauer, kernel, festevam, linux-imx,
linux-media, linux-kernel, linux-arm-kernel
Hi Ming,
Le mardi 28 juin 2022 à 13:20 +0800, Ming Qian a écrit :
> For some formats, the amphion vpu requires startcode
> before sequence and frame, such as vc1, vp8.
I'm not sure why VP8 is being mentioned here. There is no specified sartcode for
VP8, and no split of headers either.
>
> 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.
>
> Fixes: 145e936380edb ("media: amphion: implement malone decoder rpc interface")
> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> ---
> 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,
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [EXT] Re: [PATCH] media: amphion: only insert the first sequence startcode for vc1l format
2022-07-04 16:05 ` Nicolas Dufresne
@ 2022-07-05 2:00 ` Ming Qian
2022-07-05 11:27 ` Ming Qian
0 siblings, 1 reply; 11+ messages in thread
From: Ming Qian @ 2022-07-05 2:00 UTC (permalink / raw)
To: Nicolas Dufresne, mchehab, hverkuil-cisco
Cc: shawnguo, robh+dt, s.hauer, kernel, festevam, dl-linux-imx,
linux-media, linux-kernel, linux-arm-kernel
> From: Nicolas Dufresne <nicolas@ndufresne.ca>
> Sent: 2022年7月5日 0:06
> To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org;
> hverkuil-cisco@xs4all.nl
> Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> <linux-imx@nxp.com>; linux-media@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: [EXT] Re: [PATCH] media: amphion: only insert the first sequence
> startcode for vc1l format
>
> Caution: EXT Email
>
> Hi Ming,
>
> Le mardi 28 juin 2022 à 13:20 +0800, Ming Qian a écrit :
> > For some formats, the amphion vpu requires startcode before sequence
> > and frame, such as vc1, vp8.
>
> I'm not sure why VP8 is being mentioned here. There is no specified sartcode
> for VP8, and no split of headers either.
>
Hi Nicolas,
This patch has nothing to do with vp8, and indeed there is no specified startcode for VP8.
But amphion vpu requires driver to help insert some custom startcode for vp8 and vc1.
It's custom behavior.
I'm sorry that my description include some confusion
Ming
> >
> > 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.
> >
> > Fixes: 145e936380edb ("media: amphion: implement malone decoder rpc
> > interface")
> > Signed-off-by: Ming Qian <ming.qian@nxp.com>
> > ---
> > 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,
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [EXT] Re: [PATCH] media: amphion: only insert the first sequence startcode for vc1l format
2022-07-05 2:00 ` [EXT] " Ming Qian
@ 2022-07-05 11:27 ` Ming Qian
2022-07-05 12:59 ` Nicolas Dufresne
0 siblings, 1 reply; 11+ messages in thread
From: Ming Qian @ 2022-07-05 11:27 UTC (permalink / raw)
To: Nicolas Dufresne, mchehab, hverkuil-cisco
Cc: shawnguo, robh+dt, s.hauer, kernel, festevam, dl-linux-imx,
linux-media, linux-kernel, linux-arm-kernel
>>From: Ming Qian
>>Sent: 2022年7月5日 10:00
>>To: Nicolas Dufresne <nicolas@ndufresne.ca>; mchehab@kernel.org;
>>hverkuil-cisco@xs4all.nl
>>Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
>>kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
>><linux-imx@nxp.com>; linux-media@vger.kernel.org;
>>linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>>Subject: RE: [EXT] Re: [PATCH] media: amphion: only insert the first sequence
>>startcode for vc1l format
>>
>>> From: Nicolas Dufresne <nicolas@ndufresne.ca>
>>> Sent: 2022年7月5日 0:06
>>> To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org;
>>> hverkuil-cisco@xs4all.nl
>>> Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
>>> kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
>>> <linux-imx@nxp.com>; linux-media@vger.kernel.org;
>>> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>>> Subject: [EXT] Re: [PATCH] media: amphion: only insert the first
>>> sequence startcode for vc1l format
>>>
>>> Caution: EXT Email
>>>
>>> Hi Ming,
>>>
>>> Le mardi 28 juin 2022 à 13:20 +0800, Ming Qian a écrit :
>>> > For some formats, the amphion vpu requires startcode before sequence
>>> > and frame, such as vc1, vp8.
>>>
>>> I'm not sure why VP8 is being mentioned here. There is no specified
>>> sartcode for VP8, and no split of headers either.
>>>
>>
>>Hi Nicolas,
>> This patch has nothing to do with vp8, and indeed there is no specified
>>startcode for VP8.
>>But amphion vpu requires driver to help insert some custom startcode for vp8
>>and vc1.
>>It's custom behavior.
>>
> Instead of exposing the custom format, you should use data_offset like VENUS driver do. They vp8/9 codec in VENUS have the frame prefixed with an IVF header, the data_offset let the userland skip it.
Hi Nicolas,
There is a stream buffer, and driver will copy the coded frame data to the stream buffer, driver can decide to insert custom startcode or not, userland won't know about it, so I don't think it's necessary to use data_offset to let userland skip something.
Currently , driver will insert startcode for format vp8 and vc1. This is transparent to the user.
Ming
>> I'm sorry that my description include some confusion
>>
>>Ming
>>
>>> >
>>> > 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.
>>> >
>>> > Fixes: 145e936380edb ("media: amphion: implement malone decoder rpc
>>> > interface")
>>> > Signed-off-by: Ming Qian <ming.qian@nxp.com>
>>> > ---
>>> > 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,
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [EXT] Re: [PATCH] media: amphion: only insert the first sequence startcode for vc1l format
2022-07-05 11:27 ` Ming Qian
@ 2022-07-05 12:59 ` Nicolas Dufresne
2022-07-06 4:09 ` Ming Qian
0 siblings, 1 reply; 11+ messages in thread
From: Nicolas Dufresne @ 2022-07-05 12:59 UTC (permalink / raw)
To: Ming Qian, mchehab, hverkuil-cisco
Cc: shawnguo, robh+dt, s.hauer, kernel, festevam, dl-linux-imx,
linux-media, linux-kernel, linux-arm-kernel
Le mardi 05 juillet 2022 à 11:27 +0000, Ming Qian a écrit :
> > > From: Ming Qian
> > > Sent: 2022年7月5日 10:00
> > > To: Nicolas Dufresne <nicolas@ndufresne.ca>; mchehab@kernel.org;
> > > hverkuil-cisco@xs4all.nl
> > > Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
> > > kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> > > <linux-imx@nxp.com>; linux-media@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> > > Subject: RE: [EXT] Re: [PATCH] media: amphion: only insert the first
> > > sequence
> > > startcode for vc1l format
> > >
> > > > From: Nicolas Dufresne <nicolas@ndufresne.ca>
> > > > Sent: 2022年7月5日 0:06
> > > > To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org;
> > > > hverkuil-cisco@xs4all.nl
> > > > Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
> > > > kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
> > > > <linux-imx@nxp.com>; linux-media@vger.kernel.org;
> > > > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> > > > Subject: [EXT] Re: [PATCH] media: amphion: only insert the first
> > > > sequence startcode for vc1l format
> > > >
> > > > Caution: EXT Email
> > > >
> > > > Hi Ming,
> > > >
> > > > Le mardi 28 juin 2022 à 13:20 +0800, Ming Qian a écrit :
> > > > > For some formats, the amphion vpu requires startcode before sequence
> > > > > and frame, such as vc1, vp8.
> > > >
> > > > I'm not sure why VP8 is being mentioned here. There is no specified
> > > > sartcode for VP8, and no split of headers either.
> > > >
> > >
> > > Hi Nicolas,
> > > This patch has nothing to do with vp8, and indeed there is no specified
> > > startcode for VP8.
> > > But amphion vpu requires driver to help insert some custom startcode for
> > > vp8
> > > and vc1.
> > > It's custom behavior.
> > >
> > Instead of exposing the custom format, you should use data_offset like VENUS
> > driver do. They vp8/9 codec in VENUS have the frame prefixed with an IVF
> > header, the data_offset let the userland skip it.
>
> Hi Nicolas,
> There is a stream buffer, and driver will copy the coded frame data to the
> stream buffer, driver can decide to insert custom startcode or not, userland
> won't know about it, so I don't think it's necessary to use data_offset to let
> userland skip something.
> Currently , driver will insert startcode for format vp8 and vc1. This is
> transparent to the user.
Can't you save the slow copy by using data_offset then ? I think most of the
confusion comes from this commit message, someone else then you should be able
to understand what it means.
regards,
Nicolas
>
> Ming
>
> > > I'm sorry that my description include some confusion
> > >
> > > Ming
> > >
> > > > >
> > > > > 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.
> > > > >
> > > > > Fixes: 145e936380edb ("media: amphion: implement malone decoder rpc
> > > > > interface")
> > > > > Signed-off-by: Ming Qian <ming.qian@nxp.com>
> > > > > ---
> > > > > 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,
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [EXT] Re: [PATCH] media: amphion: only insert the first sequence startcode for vc1l format
2022-07-05 12:59 ` Nicolas Dufresne
@ 2022-07-06 4:09 ` Ming Qian
2022-07-08 13:32 ` Nicolas Dufresne
0 siblings, 1 reply; 11+ messages in thread
From: Ming Qian @ 2022-07-06 4:09 UTC (permalink / raw)
To: Nicolas Dufresne, mchehab, hverkuil-cisco
Cc: shawnguo, robh+dt, s.hauer, kernel, festevam, dl-linux-imx,
linux-media, linux-kernel, linux-arm-kernel
>From: Nicolas Dufresne <nicolas@ndufresne.ca>
>Sent: 2022年7月5日 20:59
>To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org;
>hverkuil-cisco@xs4all.nl
>Cc: shawnguo@kernel.org; robh+dt@kernel.org; s.hauer@pengutronix.de;
>kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx
><linux-imx@nxp.com>; linux-media@vger.kernel.org;
>linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>Subject: Re: [EXT] Re: [PATCH] media: amphion: only insert the first sequence
>startcode for vc1l format
>
>Caution: EXT Email
>
>Le mardi 05 juillet 2022 à 11:27 +0000, Ming Qian a écrit :
>> > > From: Ming Qian
>> > > Sent: 2022年7月5日 10:00
>> > > To: Nicolas Dufresne <nicolas@ndufresne.ca>; mchehab@kernel.org;
>> > > hverkuil-cisco@xs4all.nl
>> > > Cc: shawnguo@kernel.org; robh+dt@kernel.org;
>> > > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
>> > > dl-linux-imx <linux-imx@nxp.com>; linux-media@vger.kernel.org;
>> > > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>> > > Subject: RE: [EXT] Re: [PATCH] media: amphion: only insert the
>> > > first sequence startcode for vc1l format
>> > >
>> > > > From: Nicolas Dufresne <nicolas@ndufresne.ca>
>> > > > Sent: 2022年7月5日 0:06
>> > > > To: Ming Qian <ming.qian@nxp.com>; mchehab@kernel.org;
>> > > > hverkuil-cisco@xs4all.nl
>> > > > Cc: shawnguo@kernel.org; robh+dt@kernel.org;
>> > > > s.hauer@pengutronix.de; kernel@pengutronix.de;
>> > > > festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
>> > > > linux-media@vger.kernel.org; linux-kernel@vger.kernel.org;
>> > > > linux-arm-kernel@lists.infradead.org
>> > > > Subject: [EXT] Re: [PATCH] media: amphion: only insert the first
>> > > > sequence startcode for vc1l format
>> > > >
>> > > > Caution: EXT Email
>> > > >
>> > > > Hi Ming,
>> > > >
>> > > > Le mardi 28 juin 2022 à 13:20 +0800, Ming Qian a écrit :
>> > > > > For some formats, the amphion vpu requires startcode before
>> > > > > sequence and frame, such as vc1, vp8.
>> > > >
>> > > > I'm not sure why VP8 is being mentioned here. There is no
>> > > > specified sartcode for VP8, and no split of headers either.
>> > > >
>> > >
>> > > Hi Nicolas,
>> > > This patch has nothing to do with vp8, and indeed there is no
>> > > specified startcode for VP8.
>> > > But amphion vpu requires driver to help insert some custom
>> > > startcode for
>> > > vp8
>> > > and vc1.
>> > > It's custom behavior.
>> > >
>> > Instead of exposing the custom format, you should use data_offset
>> > like VENUS driver do. They vp8/9 codec in VENUS have the frame
>> > prefixed with an IVF header, the data_offset let the userland skip it.
>>
>> Hi Nicolas,
>> There is a stream buffer, and driver will copy the coded frame
>> data to the stream buffer, driver can decide to insert custom
>> startcode or not, userland won't know about it, so I don't think it's
>> necessary to use data_offset to let userland skip something.
>> Currently , driver will insert startcode for format vp8 and vc1.
>> This is transparent to the user.
>
>Can't you save the slow copy by using data_offset then ? I think most of the
>confusion comes from this commit message, someone else then you should be
>able to understand what it means.
I'll modify the commit message that remove the unrelated vp8 description.
And unfortunately the amphion vpu only support the ring buffer mode, so copying is inevitable.
>
>regards,
>Nicolas
>
>>
>> Ming
>>
>> > > I'm sorry that my description include some confusion
>> > >
>> > > Ming
>> > >
>> > > > >
>> > > > > 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.
>> > > > >
>> > > > > Fixes: 145e936380edb ("media: amphion: implement malone
>> > > > > decoder rpc
>> > > > > interface")
>> > > > > Signed-off-by: Ming Qian <ming.qian@nxp.com>
>> > > > > ---
>> > > > > 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,
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [EXT] Re: [PATCH] media: amphion: only insert the first sequence startcode for vc1l format
2022-07-06 4:09 ` Ming Qian
@ 2022-07-08 13:32 ` Nicolas Dufresne
0 siblings, 0 replies; 11+ messages in thread
From: Nicolas Dufresne @ 2022-07-08 13:32 UTC (permalink / raw)
To: Ming Qian, mchehab, hverkuil-cisco
Cc: shawnguo, robh+dt, s.hauer, kernel, festevam, dl-linux-imx,
linux-media, linux-kernel, linux-arm-kernel
Le mercredi 06 juillet 2022 à 04:09 +0000, Ming Qian a écrit :
> > Can't you save the slow copy by using data_offset then ? I think most of the
> > confusion comes from this commit message, someone else then you should be
> > able to understand what it means.
>
> I'll modify the commit message that remove the unrelated vp8 description.
> And unfortunately the amphion vpu only support the ring buffer mode, so
> copying is inevitable.
Great thanks, I had forgotten that Amphion was based on a ring buffer. Indeed,
this is the way to go, same applied to CODA driver.
Nicolas
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-07-08 13:32 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-28 5:20 [PATCH] media: amphion: only insert the first sequence startcode for vc1l format Ming Qian
2022-06-29 12:10 ` Hans Verkuil
2022-06-29 12:53 ` [EXT] " Ming Qian
2022-06-29 13:03 ` Hans Verkuil
2022-06-29 13:08 ` Ming Qian
2022-07-04 16:05 ` Nicolas Dufresne
2022-07-05 2:00 ` [EXT] " Ming Qian
2022-07-05 11:27 ` Ming Qian
2022-07-05 12:59 ` Nicolas Dufresne
2022-07-06 4:09 ` Ming Qian
2022-07-08 13:32 ` Nicolas Dufresne
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).