| Message ID | 20190822194500.2071-6-jernej.skrabec@siol.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: cedrus: h264: Support multi-slice frames | expand |
Hi Jernej, On Thu, 22 Aug 2019 21:44:57 +0200 Jernej Skrabec <jernej.skrabec@siol.net> 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 <jernej.skrabec@siol.net> > --- > 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 <jernej.skrabec@siol.net> 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 <jernej.skrabec@siol.net> > > --- > > > > 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 <jernej.skrabec@siol.net> 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 <jernej.skrabec@siol.net> > > --- > > > > 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 <jernej.skrabec@siol.net> 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 <jernej.skrabec@siol.net> 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 <jernej.skrabec@siol.net> > > > --- > > > > > > 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 <jernej.skrabec@siol.net> > --- > 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 <jernej.skrabec@siol.net> 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 <jernej.skrabec@siol.net> 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 <jernej.skrabec@siol.net> > > > > --- > > > > > > > > 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
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;
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 <jernej.skrabec@siol.net> --- drivers/staging/media/sunxi/cedrus/cedrus.h | 1 + drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++ 2 files changed, 3 insertions(+)