Message ID | 20231105165521.3592037-6-jonas@kwiboo.se |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:8f47:0:b0:403:3b70:6f57 with SMTP id j7csp2215806vqu; Sun, 5 Nov 2023 08:57:18 -0800 (PST) X-Google-Smtp-Source: AGHT+IEqqIRnyxpkp6P4Lp8d2tdaIWy0b0oRfGkoWomEVsHgBNxthoEKOhDoghtYOuiGGQ/bfmdk X-Received: by 2002:a17:902:8c85:b0:1c9:c0fa:dfb7 with SMTP id t5-20020a1709028c8500b001c9c0fadfb7mr27144468plo.57.1699203438067; Sun, 05 Nov 2023 08:57:18 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1699203438; cv=none; d=google.com; s=arc-20160816; b=d4QurncLer0a6f4XcFWQjLWu3J+Y2VAmjYnqWgo/0Tr+fz4vVc/dEDqTj3g4dvOGM/ SvvbmLKOwNWx5dzbroDakLDE98LTvLHbwSP+mzHgVOIoP/4J6/HZOmfQXT1T/6N5CLF2 s7AD/xg7KsGTCSwhLpAorpZ3OelOi6OvOiNnLgy5lXdLKoM1l82fBuNaBAwYSTH1EXAC VrDbex5Z6zxUdbC0M60P9h4TibjHhnlaPwxks0uYC96phA/4b/vZMKNo+pTczQOInp6g yaj6NeZ973Y4rxxm/A/4iGQp2wy+WfSGcVYrn8Tv076rAY8dqRabCaBleKh0AKIrHPuL KzOQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=QFwovX5RPW7IMq4uUoTrIwvqoq7/7Q8z3p4frGKGekE=; fh=ZAb5e/67NTsWML6RpVmzFEm3L9CC7ETVZIg6hSgYq5w=; b=GFzxUdNVZOTCu8zMVy3F5BajUIaIBuAH5l3cqa91B5NvIL7SKQdovAV90rIj1Bl1MN GZ6IuMrKuj2JUxp4BW6rc5kMsBK8MNDcl4JO6c+kwX2LalajiCUaz12E5BvfJ+Ujck4y CnIXsuQ5ys/NVR8brv8RgTCtGaMA+nyzmYiESKMGheU2YvnCqWBgzdjFxy/iZKzE1Ugr FV46YEpo2TzcrUqTw4F0hpi9q3o21NQ+oU4Uk0vxZajFsQkJypE8V5Sm6c4ME5Ns++x1 3TI4NZhIKrE6xaSLfDit0y+BZr1j9R0rUlBOp9kmwKvgYccQD6Fo8NGAIJX3ckbkHvQJ A2Lw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kwiboo.se header.s=fe-e1b5cab7be header.b=iogMt8BW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kwiboo.se Received: from lipwig.vger.email (lipwig.vger.email. [2620:137:e000::3:3]) by mx.google.com with ESMTPS id w18-20020a170902d11200b001cc310cb08dsi5913764plw.53.2023.11.05.08.57.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Nov 2023 08:57:18 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3; Authentication-Results: mx.google.com; dkim=pass header.i=@kwiboo.se header.s=fe-e1b5cab7be header.b=iogMt8BW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kwiboo.se Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id 9FF55808E678; Sun, 5 Nov 2023 08:56:42 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229926AbjKEQz7 (ORCPT <rfc822;heyuhang3455@gmail.com> + 34 others); Sun, 5 Nov 2023 11:55:59 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38624 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229750AbjKEQzy (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 5 Nov 2023 11:55:54 -0500 Received: from smtp.forwardemail.net (smtp.forwardemail.net [149.28.215.223]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0F582D9; Sun, 5 Nov 2023 08:55:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kwiboo.se; h=Content-Transfer-Encoding: MIME-Version: References: In-Reply-To: Message-ID: Date: Subject: Cc: To: From; q=dns/txt; s=fe-e1b5cab7be; t=1699203347; bh=QFwovX5RPW7IMq4uUoTrIwvqoq7/7Q8z3p4frGKGekE=; b=iogMt8BWbZ0IMRmVXbXItCv2PMI90AvIJerLiaBkLdr5Iz3gxsyeA+sJtYcyrGpEiAq4Cx4jh CU6e08rrf0+3hAGVy+uUEWSLW1V9mgO3K0MhMjoD5j3sNnCLB/zlyIgQKEVPNEPMsJJsCjUPrSp bkjdHkeuCSVnNC6wRPf9nVTntHFNca6abSevGZl1ivQkXbc4zjsKD0bby05TvBxzYGNDV4jJEvN msaVQ7ak2pAhjvxTFBh/wR7hcGbvTNhHB8PrknCBLCMXttDHfgNPsOPd5mIGi3gHBL284C2+afM /TMAOTHtW2vO7Dp0JJU8Pi7PNGYV/XciaErgvSuz78Hw== From: Jonas Karlman <jonas@kwiboo.se> To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>, Mauro Carvalho Chehab <mchehab@kernel.org>, Hans Verkuil <hverkuil-cisco@xs4all.nl>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> Cc: Alex Bee <knaerzche@gmail.com>, Nicolas Dufresne <nicolas.dufresne@collabora.com>, Benjamin Gaignard <benjamin.gaignard@collabora.com>, Sebastian Fricke <sebastian.fricke@collabora.com>, Christopher Obbard <chris.obbard@collabora.com>, linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org, Jonas Karlman <jonas@kwiboo.se> Subject: [PATCH v4 05/11] media: rkvdec: h264: Remove SPS validation at streaming start Date: Sun, 5 Nov 2023 16:55:04 +0000 Message-ID: <20231105165521.3592037-6-jonas@kwiboo.se> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231105165521.3592037-1-jonas@kwiboo.se> References: <20231105165521.3592037-1-jonas@kwiboo.se> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Report-Abuse-To: abuse@forwardemail.net X-Report-Abuse: abuse@forwardemail.net X-Complaints-To: abuse@forwardemail.net X-ForwardEmail-Version: 0.4.40 X-ForwardEmail-Sender: rfc822; jonas@kwiboo.se, smtp.forwardemail.net, 149.28.215.223 X-ForwardEmail-ID: 6547c91342ad2f8d1524691b X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Sun, 05 Nov 2023 08:56:42 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1781743944200122188 X-GMAIL-MSGID: 1781743944200122188 |
Series |
media: rkvdec: Add H.264 High 10 and 4:2:2 profile support
|
|
Commit Message
Jonas Karlman
Nov. 5, 2023, 4:55 p.m. UTC
SPS parameters is validated in try_ctrl() ops so there is no need to
re-validate when streaming starts.
Remove the unnecessary call to validate sps at streaming start.
Suggested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v4:
- No change
v3:
- New patch
drivers/staging/media/rkvdec/rkvdec-h264.c | 19 ++-----------------
1 file changed, 2 insertions(+), 17 deletions(-)
Comments
Le dimanche 05 novembre 2023 à 16:55 +0000, Jonas Karlman a écrit : > SPS parameters is validated in try_ctrl() ops so there is no need to are > re-validate when streaming starts. > > Remove the unnecessary call to validate sps at streaming start. This patch is not working since user may change the format after the control have been set. The proper fix should actually reset the SPS (well all header controls) to match the the newly set format. Then this validation won't be needed anymore. The sequence that is problematic after this patch is: S_FMT (OUTPUT, width, height); S_CTRL (SPS) // valid S_FMT(OUTPUT, width', height'); // SPS is no longer valid One suggestion I may make is to add a ops so that each codec implementation can reset their header controls to make it valid against the new resolution. With that in place you'll be able drop the check. Nicolas p.s. you can also just drop this patch from the series and revisit it later, though I think its worth fixing. > > Suggested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > --- > v4: > - No change > > v3: > - New patch > > drivers/staging/media/rkvdec/rkvdec-h264.c | 19 ++----------------- > 1 file changed, 2 insertions(+), 17 deletions(-) > > diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c > index 8bce8902b8dd..815d5359ddd5 100644 > --- a/drivers/staging/media/rkvdec/rkvdec-h264.c > +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c > @@ -1070,17 +1070,6 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) > struct rkvdec_dev *rkvdec = ctx->dev; > struct rkvdec_h264_priv_tbl *priv_tbl; > struct rkvdec_h264_ctx *h264_ctx; > - struct v4l2_ctrl *ctrl; > - int ret; > - > - ctrl = v4l2_ctrl_find(&ctx->ctrl_hdl, > - V4L2_CID_STATELESS_H264_SPS); > - if (!ctrl) > - return -EINVAL; > - > - ret = rkvdec_h264_validate_sps(ctx, ctrl->p_new.p_h264_sps); > - if (ret) > - return ret; > > h264_ctx = kzalloc(sizeof(*h264_ctx), GFP_KERNEL); > if (!h264_ctx) > @@ -1089,8 +1078,8 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) > priv_tbl = dma_alloc_coherent(rkvdec->dev, sizeof(*priv_tbl), > &h264_ctx->priv_tbl.dma, GFP_KERNEL); > if (!priv_tbl) { > - ret = -ENOMEM; > - goto err_free_ctx; > + kfree(h264_ctx); > + return -ENOMEM; > } > > h264_ctx->priv_tbl.size = sizeof(*priv_tbl); > @@ -1100,10 +1089,6 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) > > ctx->priv = h264_ctx; > return 0; > - > -err_free_ctx: > - kfree(h264_ctx); > - return ret; > } > > static void rkvdec_h264_stop(struct rkvdec_ctx *ctx)
On 2023-11-07 23:01, Nicolas Dufresne wrote: > Le dimanche 05 novembre 2023 à 16:55 +0000, Jonas Karlman a écrit : >> SPS parameters is validated in try_ctrl() ops so there is no need to > > are > >> re-validate when streaming starts. >> >> Remove the unnecessary call to validate sps at streaming start. > > This patch is not working since user may change the format after the > control have been set. The proper fix should actually reset the SPS > (well all header controls) to match the the newly set format. Then this > validation won't be needed anymore. > > The sequence that is problematic after this patch is: > > S_FMT (OUTPUT, width, height); > S_CTRL (SPS) // valid > S_FMT(OUTPUT, width', height'); // SPS is no longer valid > > One suggestion I may make is to add a ops so that each codec > implementation can reset their header controls to make it valid against > the new resolution. With that in place you'll be able drop the check. According to the Initialization section of the V4L2 stateless documentation a client is supposed to S_CTRL(SPS) after S_FMT(OUTPUT). https://docs.kernel.org/userspace-api/media/v4l/dev-stateless-decoder.html#initialization I guess that all stateless decoders probably should reset all ctrls to default value on S_FMT(OUTPUT) or decoders may end up in an unexpected state? Is resetting a ctrl value back to default something that is supported by v4l2 ctrl core? Did not find any obvious way to reset a ctrl value. Will probably just drop this patch in v5. Regards, Jonas > > Nicolas > > p.s. you can also just drop this patch from the series and revisit it > later, though I think its worth fixing. > >> >> Suggested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> >> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >> --- >> v4: >> - No change >> >> v3: >> - New patch >> >> drivers/staging/media/rkvdec/rkvdec-h264.c | 19 ++----------------- >> 1 file changed, 2 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c >> index 8bce8902b8dd..815d5359ddd5 100644 >> --- a/drivers/staging/media/rkvdec/rkvdec-h264.c >> +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c >> @@ -1070,17 +1070,6 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) >> struct rkvdec_dev *rkvdec = ctx->dev; >> struct rkvdec_h264_priv_tbl *priv_tbl; >> struct rkvdec_h264_ctx *h264_ctx; >> - struct v4l2_ctrl *ctrl; >> - int ret; >> - >> - ctrl = v4l2_ctrl_find(&ctx->ctrl_hdl, >> - V4L2_CID_STATELESS_H264_SPS); >> - if (!ctrl) >> - return -EINVAL; >> - >> - ret = rkvdec_h264_validate_sps(ctx, ctrl->p_new.p_h264_sps); >> - if (ret) >> - return ret; >> >> h264_ctx = kzalloc(sizeof(*h264_ctx), GFP_KERNEL); >> if (!h264_ctx) >> @@ -1089,8 +1078,8 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) >> priv_tbl = dma_alloc_coherent(rkvdec->dev, sizeof(*priv_tbl), >> &h264_ctx->priv_tbl.dma, GFP_KERNEL); >> if (!priv_tbl) { >> - ret = -ENOMEM; >> - goto err_free_ctx; >> + kfree(h264_ctx); >> + return -ENOMEM; >> } >> >> h264_ctx->priv_tbl.size = sizeof(*priv_tbl); >> @@ -1100,10 +1089,6 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) >> >> ctx->priv = h264_ctx; >> return 0; >> - >> -err_free_ctx: >> - kfree(h264_ctx); >> - return ret; >> } >> >> static void rkvdec_h264_stop(struct rkvdec_ctx *ctx) >
Le mardi 07 novembre 2023 à 23:56 +0100, Jonas Karlman a écrit : > On 2023-11-07 23:01, Nicolas Dufresne wrote: > > Le dimanche 05 novembre 2023 à 16:55 +0000, Jonas Karlman a écrit : > > > SPS parameters is validated in try_ctrl() ops so there is no need to > > > > are > > > > > re-validate when streaming starts. > > > > > > Remove the unnecessary call to validate sps at streaming start. > > > > This patch is not working since user may change the format after the > > control have been set. The proper fix should actually reset the SPS > > (well all header controls) to match the the newly set format. Then this > > validation won't be needed anymore. > > > > The sequence that is problematic after this patch is: > > > > S_FMT (OUTPUT, width, height); > > S_CTRL (SPS) // valid > > S_FMT(OUTPUT, width', height'); // SPS is no longer valid > > > > One suggestion I may make is to add a ops so that each codec > > implementation can reset their header controls to make it valid against > > the new resolution. With that in place you'll be able drop the check. > > According to the Initialization section of the V4L2 stateless > documentation a client is supposed to S_CTRL(SPS) after S_FMT(OUTPUT). Yes, but other part of the spec prevents us from failing if the userspace restart in the middle of the process. > > https://docs.kernel.org/userspace-api/media/v4l/dev-stateless-decoder.html#initialization > > I guess that all stateless decoders probably should reset all ctrls to > default value on S_FMT(OUTPUT) or decoders may end up in an unexpected > state? > > Is resetting a ctrl value back to default something that is supported by > v4l2 ctrl core? Did not find any obvious way to reset a ctrl value. In order to avoid having to do this, Hantro driver just ignores these values from SPS control and do the following: reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(MB_WIDTH(ctx->src_fmt.width)) | G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->src_fmt.height)) | G1_REG_DEC_CTRL1_REF_FRAMES(sps->max_num_ref_frames); Then all they do is reset the CAPTURE format whenever needed, matching the bit depth that was previously set. The SPS is unfortunatly not guarantied to be valid, but at first sight its safe in regard to picture dimensions. > > Will probably just drop this patch in v5. That or do like in Hantro driver. What is scary though is that some of the feature enabled by SPS may requires an auxiliary chunk of memory to be allocated, and then this method falls appart. I think it would be nice to fix that properly in all drivers in a future patchset. > > Regards, > Jonas > > > > > Nicolas > > > > p.s. you can also just drop this patch from the series and revisit it > > later, though I think its worth fixing. > > > > > > > > Suggested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > > > --- > > > v4: > > > - No change > > > > > > v3: > > > - New patch > > > > > > drivers/staging/media/rkvdec/rkvdec-h264.c | 19 ++----------------- > > > 1 file changed, 2 insertions(+), 17 deletions(-) > > > > > > diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c > > > index 8bce8902b8dd..815d5359ddd5 100644 > > > --- a/drivers/staging/media/rkvdec/rkvdec-h264.c > > > +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c > > > @@ -1070,17 +1070,6 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) > > > struct rkvdec_dev *rkvdec = ctx->dev; > > > struct rkvdec_h264_priv_tbl *priv_tbl; > > > struct rkvdec_h264_ctx *h264_ctx; > > > - struct v4l2_ctrl *ctrl; > > > - int ret; > > > - > > > - ctrl = v4l2_ctrl_find(&ctx->ctrl_hdl, > > > - V4L2_CID_STATELESS_H264_SPS); > > > - if (!ctrl) > > > - return -EINVAL; > > > - > > > - ret = rkvdec_h264_validate_sps(ctx, ctrl->p_new.p_h264_sps); > > > - if (ret) > > > - return ret; > > > > > > h264_ctx = kzalloc(sizeof(*h264_ctx), GFP_KERNEL); > > > if (!h264_ctx) > > > @@ -1089,8 +1078,8 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) > > > priv_tbl = dma_alloc_coherent(rkvdec->dev, sizeof(*priv_tbl), > > > &h264_ctx->priv_tbl.dma, GFP_KERNEL); > > > if (!priv_tbl) { > > > - ret = -ENOMEM; > > > - goto err_free_ctx; > > > + kfree(h264_ctx); > > > + return -ENOMEM; > > > } > > > > > > h264_ctx->priv_tbl.size = sizeof(*priv_tbl); > > > @@ -1100,10 +1089,6 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) > > > > > > ctx->priv = h264_ctx; > > > return 0; > > > - > > > -err_free_ctx: > > > - kfree(h264_ctx); > > > - return ret; > > > } > > > > > > static void rkvdec_h264_stop(struct rkvdec_ctx *ctx) > > >
On 2023-11-08 03:39, Nicolas Dufresne wrote: > Le mardi 07 novembre 2023 à 23:56 +0100, Jonas Karlman a écrit : >> On 2023-11-07 23:01, Nicolas Dufresne wrote: >>> Le dimanche 05 novembre 2023 à 16:55 +0000, Jonas Karlman a écrit : >>>> SPS parameters is validated in try_ctrl() ops so there is no need to >>> >>> are >>> >>>> re-validate when streaming starts. >>>> >>>> Remove the unnecessary call to validate sps at streaming start. >>> >>> This patch is not working since user may change the format after the >>> control have been set. The proper fix should actually reset the SPS >>> (well all header controls) to match the the newly set format. Then this >>> validation won't be needed anymore. >>> >>> The sequence that is problematic after this patch is: >>> >>> S_FMT (OUTPUT, width, height); >>> S_CTRL (SPS) // valid >>> S_FMT(OUTPUT, width', height'); // SPS is no longer valid >>> >>> One suggestion I may make is to add a ops so that each codec >>> implementation can reset their header controls to make it valid against >>> the new resolution. With that in place you'll be able drop the check. >> >> According to the Initialization section of the V4L2 stateless >> documentation a client is supposed to S_CTRL(SPS) after S_FMT(OUTPUT). > > Yes, but other part of the spec prevents us from failing if the > userspace restart in the middle of the process. > >> >> https://docs.kernel.org/userspace-api/media/v4l/dev-stateless-decoder.html#initialization >> >> I guess that all stateless decoders probably should reset all ctrls to >> default value on S_FMT(OUTPUT) or decoders may end up in an unexpected >> state? >> >> Is resetting a ctrl value back to default something that is supported by >> v4l2 ctrl core? Did not find any obvious way to reset a ctrl value. > > In order to avoid having to do this, Hantro driver just ignores these > values from SPS control and do the following: > > reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(MB_WIDTH(ctx->src_fmt.width)) | > G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->src_fmt.height)) | > G1_REG_DEC_CTRL1_REF_FRAMES(sps->max_num_ref_frames); > > Then all they do is reset the CAPTURE format whenever needed, matching > the bit depth that was previously set. The SPS is unfortunatly not > guarantied to be valid, but at first sight its safe in regard to > picture dimensions. The commit 77e74be83083 ("media: rkvdec: h264: Validate and use pic width and height in mbs") changed to use the SPS values to help fix decoding of field encoded content, it also added this check. Will drop this patch in v5, and should also re-add similar validation in the HEVC series. Regards, Jonas > >> >> Will probably just drop this patch in v5. > > That or do like in Hantro driver. What is scary though is that some of > the feature enabled by SPS may requires an auxiliary chunk of memory to > be allocated, and then this method falls appart. I think it would be > nice to fix that properly in all drivers in a future patchset. > >> >> Regards, >> Jonas >> >>> >>> Nicolas >>> >>> p.s. you can also just drop this patch from the series and revisit it >>> later, though I think its worth fixing. >>> >>>> >>>> Suggested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> >>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se> >>>> --- >>>> v4: >>>> - No change >>>> >>>> v3: >>>> - New patch >>>> >>>> drivers/staging/media/rkvdec/rkvdec-h264.c | 19 ++----------------- >>>> 1 file changed, 2 insertions(+), 17 deletions(-) >>>> >>>> diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c >>>> index 8bce8902b8dd..815d5359ddd5 100644 >>>> --- a/drivers/staging/media/rkvdec/rkvdec-h264.c >>>> +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c >>>> @@ -1070,17 +1070,6 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) >>>> struct rkvdec_dev *rkvdec = ctx->dev; >>>> struct rkvdec_h264_priv_tbl *priv_tbl; >>>> struct rkvdec_h264_ctx *h264_ctx; >>>> - struct v4l2_ctrl *ctrl; >>>> - int ret; >>>> - >>>> - ctrl = v4l2_ctrl_find(&ctx->ctrl_hdl, >>>> - V4L2_CID_STATELESS_H264_SPS); >>>> - if (!ctrl) >>>> - return -EINVAL; >>>> - >>>> - ret = rkvdec_h264_validate_sps(ctx, ctrl->p_new.p_h264_sps); >>>> - if (ret) >>>> - return ret; >>>> >>>> h264_ctx = kzalloc(sizeof(*h264_ctx), GFP_KERNEL); >>>> if (!h264_ctx) >>>> @@ -1089,8 +1078,8 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) >>>> priv_tbl = dma_alloc_coherent(rkvdec->dev, sizeof(*priv_tbl), >>>> &h264_ctx->priv_tbl.dma, GFP_KERNEL); >>>> if (!priv_tbl) { >>>> - ret = -ENOMEM; >>>> - goto err_free_ctx; >>>> + kfree(h264_ctx); >>>> + return -ENOMEM; >>>> } >>>> >>>> h264_ctx->priv_tbl.size = sizeof(*priv_tbl); >>>> @@ -1100,10 +1089,6 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) >>>> >>>> ctx->priv = h264_ctx; >>>> return 0; >>>> - >>>> -err_free_ctx: >>>> - kfree(h264_ctx); >>>> - return ret; >>>> } >>>> >>>> static void rkvdec_h264_stop(struct rkvdec_ctx *ctx) >>> >> >
diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c index 8bce8902b8dd..815d5359ddd5 100644 --- a/drivers/staging/media/rkvdec/rkvdec-h264.c +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c @@ -1070,17 +1070,6 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) struct rkvdec_dev *rkvdec = ctx->dev; struct rkvdec_h264_priv_tbl *priv_tbl; struct rkvdec_h264_ctx *h264_ctx; - struct v4l2_ctrl *ctrl; - int ret; - - ctrl = v4l2_ctrl_find(&ctx->ctrl_hdl, - V4L2_CID_STATELESS_H264_SPS); - if (!ctrl) - return -EINVAL; - - ret = rkvdec_h264_validate_sps(ctx, ctrl->p_new.p_h264_sps); - if (ret) - return ret; h264_ctx = kzalloc(sizeof(*h264_ctx), GFP_KERNEL); if (!h264_ctx) @@ -1089,8 +1078,8 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) priv_tbl = dma_alloc_coherent(rkvdec->dev, sizeof(*priv_tbl), &h264_ctx->priv_tbl.dma, GFP_KERNEL); if (!priv_tbl) { - ret = -ENOMEM; - goto err_free_ctx; + kfree(h264_ctx); + return -ENOMEM; } h264_ctx->priv_tbl.size = sizeof(*priv_tbl); @@ -1100,10 +1089,6 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx) ctx->priv = h264_ctx; return 0; - -err_free_ctx: - kfree(h264_ctx); - return ret; } static void rkvdec_h264_stop(struct rkvdec_ctx *ctx)