Messages in this thread | | | Subject | Re: [PATCH 3/6] hantro: Rework how encoder and decoder are identified | From | Robin Murphy <> | Date | Fri, 26 Jun 2020 10:52:23 +0100 |
| |
Hi Ezequiel,
On 2020-06-25 17:35, Ezequiel Garcia wrote: > So far we've been using the .buf_finish hook to distinguish > decoder from encoder. This is unnecessarily obfuscated. > > Moreover, we want to move the buf_finish, so use a cleaner > scheme to distinguish the driver decoder/encoder type. > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > --- > drivers/staging/media/hantro/hantro.h | 2 ++ > drivers/staging/media/hantro/hantro_drv.c | 4 +++- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h > index 3005207fc6fb..028b788ad50f 100644 > --- a/drivers/staging/media/hantro/hantro.h > +++ b/drivers/staging/media/hantro/hantro.h > @@ -199,6 +199,7 @@ struct hantro_dev { > * > * @dev: VPU driver data to which the context belongs. > * @fh: V4L2 file handler. > + * @is_encoder: Decoder or encoder context? > * > * @sequence_cap: Sequence counter for capture queue > * @sequence_out: Sequence counter for output queue > @@ -223,6 +224,7 @@ struct hantro_dev { > struct hantro_ctx { > struct hantro_dev *dev; > struct v4l2_fh fh; > + bool is_encoder; > > u32 sequence_cap; > u32 sequence_out; > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c > index 0db8ad455160..112ed556eb90 100644 > --- a/drivers/staging/media/hantro/hantro_drv.c > +++ b/drivers/staging/media/hantro/hantro_drv.c > @@ -197,7 +197,7 @@ static void device_run(void *priv) > > bool hantro_is_encoder_ctx(const struct hantro_ctx *ctx) > { > - return ctx->buf_finish == hantro_enc_buf_finish; > + return ctx->is_encoder;
FWIW I'd suggest removing the wrapper function entirely now - it makes sense when the check itself is implemented in a weird and non-obvious way, but a simple boolean flag named exactly what it means is already about as clear as it can get.
Robin.
> } > > static struct v4l2_m2m_ops vpu_m2m_ops = { > @@ -420,8 +420,10 @@ static int hantro_open(struct file *filp) > if (func->id == MEDIA_ENT_F_PROC_VIDEO_ENCODER) { > allowed_codecs = vpu->variant->codec & HANTRO_ENCODERS; > ctx->buf_finish = hantro_enc_buf_finish; > + ctx->is_encoder = true; > } else if (func->id == MEDIA_ENT_F_PROC_VIDEO_DECODER) { > allowed_codecs = vpu->variant->codec & HANTRO_DECODERS; > + ctx->is_encoder = false; > } else { > ret = -ENODEV; > goto err_ctx_free; >
| |