Re: [PATCH 5/8] media: cedrus: Detect first slice of a frame

[Boris Brezillon]

On Thu, 29 Aug 2019 21:04:28 +0200
Jernej Åkrabec <jernej.skrabec@xxxxxxxx> 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@xxxxxxxx> 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@xxxxxxxx>
> > > ---
> > > 
> > >  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.