When codec supports multiple slices in one frame, VPU has to know when
first slice of each frame is being processed, presumably to correctly
clear/set data in auxiliary buffers.
Add first_slice field to cedrus_run structure and set it according to
timestamps of capture and output buffers. If timestamps are different,
it's first slice and viceversa.
Signed-off-by: Jernej Skrabec <[email protected]>
---
drivers/staging/media/sunxi/cedrus/cedrus.h | 1 +
drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 2f017a651848..32cb38e541c6 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -70,6 +70,7 @@ struct cedrus_mpeg2_run {
struct cedrus_run {
struct vb2_v4l2_buffer *src;
struct vb2_v4l2_buffer *dst;
+ bool first_slice;
union {
struct cedrus_h264_run h264;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
index 56ca4c9ad01c..d7b54accfe83 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
@@ -31,6 +31,8 @@ void cedrus_device_run(void *priv)
run.src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
run.dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+ run.first_slice =
+ run.src->vb2_buf.timestamp != run.dst->vb2_buf.timestamp;
/* Apply request(s) controls if needed. */
src_req = run.src->vb2_buf.req_obj.req;
--
2.22.1
Hi Jernej,
On Thu, 22 Aug 2019 21:44:57 +0200
Jernej Skrabec <[email protected]> wrote:
> When codec supports multiple slices in one frame, VPU has to know when
> first slice of each frame is being processed, presumably to correctly
> clear/set data in auxiliary buffers.
>
> Add first_slice field to cedrus_run structure and set it according to
> timestamps of capture and output buffers. If timestamps are different,
> it's first slice and viceversa.
>
> Signed-off-by: Jernej Skrabec <[email protected]>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus.h | 1 +
> drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index 2f017a651848..32cb38e541c6 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -70,6 +70,7 @@ struct cedrus_mpeg2_run {
> struct cedrus_run {
> struct vb2_v4l2_buffer *src;
> struct vb2_v4l2_buffer *dst;
> + bool first_slice;
>
> union {
> struct cedrus_h264_run h264;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> index 56ca4c9ad01c..d7b54accfe83 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> @@ -31,6 +31,8 @@ void cedrus_device_run(void *priv)
>
> run.src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> run.dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> + run.first_slice =
> + run.src->vb2_buf.timestamp != run.dst->vb2_buf.timestamp;
Can't we use slice->first_mb_in_slice to determine if a slice is the
first? I'd expect ->first_mb_in_slice to be 0 (unless we decide to
support ASO).
>
> /* Apply request(s) controls if needed. */
> src_req = run.src->vb2_buf.req_obj.req;
Dne ponedeljek, 26. avgust 2019 ob 20:28:31 CEST je Boris Brezillon
napisal(a):
> Hi Jernej,
>
> On Thu, 22 Aug 2019 21:44:57 +0200
>
> Jernej Skrabec <[email protected]> wrote:
> > When codec supports multiple slices in one frame, VPU has to know when
> > first slice of each frame is being processed, presumably to correctly
> > clear/set data in auxiliary buffers.
> >
> > Add first_slice field to cedrus_run structure and set it according to
> > timestamps of capture and output buffers. If timestamps are different,
> > it's first slice and viceversa.
> >
> > Signed-off-by: Jernej Skrabec <[email protected]>
> > ---
> >
> > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 +
> > drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
> > 2 files changed, 3 insertions(+)
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > b/drivers/staging/media/sunxi/cedrus/cedrus.h index
> > 2f017a651848..32cb38e541c6 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> > @@ -70,6 +70,7 @@ struct cedrus_mpeg2_run {
> >
> > struct cedrus_run {
> >
> > struct vb2_v4l2_buffer *src;
> > struct vb2_v4l2_buffer *dst;
> >
> > + bool first_slice;
> >
> > union {
> >
> > struct cedrus_h264_run h264;
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index
> > 56ca4c9ad01c..d7b54accfe83 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > @@ -31,6 +31,8 @@ void cedrus_device_run(void *priv)
> >
> > run.src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > run.dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> >
> > + run.first_slice =
> > + run.src->vb2_buf.timestamp != run.dst-
>vb2_buf.timestamp;
>
> Can't we use slice->first_mb_in_slice to determine if a slice is the
> first? I'd expect ->first_mb_in_slice to be 0 (unless we decide to
> support ASO).
I'm not sure if that is always the case, I would have to check the standard.
Anyway, this method of comparing timestamps was suggested to me a while ago
when we were discussing details on a way forward for multi-slice decoding. I
highly doubt someone would decode slices in mixed order (from different frames)
in same instance.
I can change that in next version if ->first_mb_in_slice == 0 is always true
for the first slice.
Best regards,
Jernej
>
> > /* Apply request(s) controls if needed. */
> > src_req = run.src->vb2_buf.req_obj.req;
Dne ponedeljek, 26. avgust 2019 ob 20:28:31 CEST je Boris Brezillon
napisal(a):
> Hi Jernej,
>
> On Thu, 22 Aug 2019 21:44:57 +0200
>
> Jernej Skrabec <[email protected]> wrote:
> > When codec supports multiple slices in one frame, VPU has to know when
> > first slice of each frame is being processed, presumably to correctly
> > clear/set data in auxiliary buffers.
> >
> > Add first_slice field to cedrus_run structure and set it according to
> > timestamps of capture and output buffers. If timestamps are different,
> > it's first slice and viceversa.
> >
> > Signed-off-by: Jernej Skrabec <[email protected]>
> > ---
> >
> > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 +
> > drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
> > 2 files changed, 3 insertions(+)
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > b/drivers/staging/media/sunxi/cedrus/cedrus.h index
> > 2f017a651848..32cb38e541c6 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> > @@ -70,6 +70,7 @@ struct cedrus_mpeg2_run {
> >
> > struct cedrus_run {
> >
> > struct vb2_v4l2_buffer *src;
> > struct vb2_v4l2_buffer *dst;
> >
> > + bool first_slice;
> >
> > union {
> >
> > struct cedrus_h264_run h264;
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index
> > 56ca4c9ad01c..d7b54accfe83 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > @@ -31,6 +31,8 @@ void cedrus_device_run(void *priv)
> >
> > run.src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > run.dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> >
> > + run.first_slice =
> > + run.src->vb2_buf.timestamp != run.dst-
>vb2_buf.timestamp;
>
> Can't we use slice->first_mb_in_slice to determine if a slice is the
> first? I'd expect ->first_mb_in_slice to be 0 (unless we decide to
> support ASO).
I looked in all VPU documentation available to me (which isn't much) and there
is no indication if ASO is supported or not. Do you have any sample video with
out-of-order slices? It's my understanding that this is uncommon. If it's
supported, I would leave code as-is.
Best regards,
Jernej
>
> > /* Apply request(s) controls if needed. */
> > src_req = run.src->vb2_buf.req_obj.req;
On Thu, 29 Aug 2019 21:04:28 +0200
Jernej Škrabec <[email protected]> wrote:
> Dne ponedeljek, 26. avgust 2019 ob 20:28:31 CEST je Boris Brezillon
> napisal(a):
> > Hi Jernej,
> >
> > On Thu, 22 Aug 2019 21:44:57 +0200
> >
> > Jernej Skrabec <[email protected]> wrote:
> > > When codec supports multiple slices in one frame, VPU has to know when
> > > first slice of each frame is being processed, presumably to correctly
> > > clear/set data in auxiliary buffers.
> > >
> > > Add first_slice field to cedrus_run structure and set it according to
> > > timestamps of capture and output buffers. If timestamps are different,
> > > it's first slice and viceversa.
> > >
> > > Signed-off-by: Jernej Skrabec <[email protected]>
> > > ---
> > >
> > > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 +
> > > drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
> > > 2 files changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > > b/drivers/staging/media/sunxi/cedrus/cedrus.h index
> > > 2f017a651848..32cb38e541c6 100644
> > > --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> > > @@ -70,6 +70,7 @@ struct cedrus_mpeg2_run {
> > >
> > > struct cedrus_run {
> > >
> > > struct vb2_v4l2_buffer *src;
> > > struct vb2_v4l2_buffer *dst;
> > >
> > > + bool first_slice;
> > >
> > > union {
> > >
> > > struct cedrus_h264_run h264;
> > >
> > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > > b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index
> > > 56ca4c9ad01c..d7b54accfe83 100644
> > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > > @@ -31,6 +31,8 @@ void cedrus_device_run(void *priv)
> > >
> > > run.src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > > run.dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > >
> > > + run.first_slice =
> > > + run.src->vb2_buf.timestamp != run.dst-
> >vb2_buf.timestamp;
> >
> > Can't we use slice->first_mb_in_slice to determine if a slice is the
> > first? I'd expect ->first_mb_in_slice to be 0 (unless we decide to
> > support ASO).
>
> I looked in all VPU documentation available to me (which isn't much) and there
> is no indication if ASO is supported or not. Do you have any sample video with
> out-of-order slices? It's my understanding that this is uncommon.
I'm not entirely sure, but my understanding was that it might be used
when streaming over network where some packets might be lost and
re-emitted later on.
> If it's
> supported, I would leave code as-is.
I remember seeing the ASO acronym mentioned in the hantro G1 spec, but
at the same time we're doing frame-based decoding, so I guess the HW
block expects slices to be ordered in that case. Honestly I don't know,
so let's keep the code as-is.
On 8/22/19 9:44 PM, Jernej Skrabec wrote:
> When codec supports multiple slices in one frame, VPU has to know when
> first slice of each frame is being processed, presumably to correctly
> clear/set data in auxiliary buffers.
>
> Add first_slice field to cedrus_run structure and set it according to
> timestamps of capture and output buffers. If timestamps are different,
> it's first slice and viceversa.
>
> Signed-off-by: Jernej Skrabec <[email protected]>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus.h | 1 +
> drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index 2f017a651848..32cb38e541c6 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -70,6 +70,7 @@ struct cedrus_mpeg2_run {
> struct cedrus_run {
> struct vb2_v4l2_buffer *src;
> struct vb2_v4l2_buffer *dst;
> + bool first_slice;
>
> union {
> struct cedrus_h264_run h264;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> index 56ca4c9ad01c..d7b54accfe83 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> @@ -31,6 +31,8 @@ void cedrus_device_run(void *priv)
>
> run.src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> run.dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> + run.first_slice =
> + run.src->vb2_buf.timestamp != run.dst->vb2_buf.timestamp;
This is almost correct. To handle the corner case where no timestamp
was ever copied to run.dst->vb2_buf you need this check:
run.first_slice = !run.dst->vb2_buf.copied_timestamp ||
run.src->vb2_buf.timestamp != run.dst->vb2_buf.timestamp;
Regards,
Hans
>
> /* Apply request(s) controls if needed. */
> src_req = run.src->vb2_buf.req_obj.req;
>
Le vendredi 30 août 2019 à 07:48 +0200, Boris Brezillon a écrit :
> On Thu, 29 Aug 2019 21:04:28 +0200
> Jernej Škrabec <[email protected]> wrote:
>
> > Dne ponedeljek, 26. avgust 2019 ob 20:28:31 CEST je Boris Brezillon
> > napisal(a):
> > > Hi Jernej,
> > >
> > > On Thu, 22 Aug 2019 21:44:57 +0200
> > >
> > > Jernej Skrabec <[email protected]> wrote:
> > > > When codec supports multiple slices in one frame, VPU has to know when
> > > > first slice of each frame is being processed, presumably to correctly
> > > > clear/set data in auxiliary buffers.
> > > >
> > > > Add first_slice field to cedrus_run structure and set it according to
> > > > timestamps of capture and output buffers. If timestamps are different,
> > > > it's first slice and viceversa.
> > > >
> > > > Signed-off-by: Jernej Skrabec <[email protected]>
> > > > ---
> > > >
> > > > drivers/staging/media/sunxi/cedrus/cedrus.h | 1 +
> > > > drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++
> > > > 2 files changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > > > b/drivers/staging/media/sunxi/cedrus/cedrus.h index
> > > > 2f017a651848..32cb38e541c6 100644
> > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> > > > @@ -70,6 +70,7 @@ struct cedrus_mpeg2_run {
> > > >
> > > > struct cedrus_run {
> > > >
> > > > struct vb2_v4l2_buffer *src;
> > > > struct vb2_v4l2_buffer *dst;
> > > >
> > > > + bool first_slice;
> > > >
> > > > union {
> > > >
> > > > struct cedrus_h264_run h264;
> > > >
> > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > > > b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index
> > > > 56ca4c9ad01c..d7b54accfe83 100644
> > > > --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > > > @@ -31,6 +31,8 @@ void cedrus_device_run(void *priv)
> > > >
> > > > run.src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > > > run.dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > > >
> > > > + run.first_slice =
> > > > + run.src->vb2_buf.timestamp != run.dst-
> > > vb2_buf.timestamp;
> > >
> > > Can't we use slice->first_mb_in_slice to determine if a slice is the
> > > first? I'd expect ->first_mb_in_slice to be 0 (unless we decide to
> > > support ASO).
> >
> > I looked in all VPU documentation available to me (which isn't much) and there
> > is no indication if ASO is supported or not. Do you have any sample video with
> > out-of-order slices? It's my understanding that this is uncommon.
>
> I'm not entirely sure, but my understanding was that it might be used
> when streaming over network where some packets might be lost and
> re-emitted later on.
>
> > If it's
> > supported, I would leave code as-is.
>
> I remember seeing the ASO acronym mentioned in the hantro G1 spec, but
> at the same time we're doing frame-based decoding, so I guess the HW
> block expects slices to be ordered in that case. Honestly I don't know,
> so let's keep the code as-is.
We had an ASO interrupt when we tried to do slice decoding rather then
frame. I believe on Hantro, the way to do ASO is to actually re-order
in software.
ASO is a feature of baseline profile use to reduce latency. As an
example, VA-API does not support baseline profile (only constrained-
baseline, which excludes ASO).
Nicolas