Received-SPF: pass (google.com: domain of linux-kernel+bounces-105898-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240221-uvc-host-video-decode-start-v3-1-40e9b9ced97b@pengutronix.de>
In-Reply-To: <20240221-uvc-host-video-decode-start-v3-1-40e9b9ced97b@pengutronix.de>
From: Ricardo Ribalda <ribalda@chromium.org>
Date: Mon, 18 Mar 2024 10:43:16 +0100
Message-ID: <CANiDSCv13TmKFNKP6khYh7cCv=mOrhVKGJ79H9knYuB6iJKqDA@mail.gmail.com>
Subject: Re: [PATCH v3] uvc_video: move clock_decode and stats_decode to the
 end of decode_start
To: Michael Grzeschik <m.grzeschik@pengutronix.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>, 
	Mauro Carvalho Chehab <mchehab@kernel.org>, linux-media@vger.kernel.org, 
	linux-kernel@vger.kernel.org, kernel@pengutronix.de
Content-Type: text/plain; charset="UTF-8"

Hi Michael

On Mon, 18 Mar 2024 at 10:28, Michael Grzeschik
<m.grzeschik@pengutronix.de> wrote:
>
> When the uvc request will get parsed by uvc_video_decode_start it will
> leave the function with -EAGAIN to be restarted on the next frame. While
> the first wrong parse the statistics will already be updated with
> uvc_video_stats_decode.
>
> One value e.g. is the error_count, which therefor will be incremented
> twice in case the fid has changed on the way. This patch fixes the
> unnecessary extra parsing by moving the decode functions to the
> end of decode_start, when it is save to really parse the data.
nit: safe
>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> Changes in v3:
> - Moved the clock_decode and stat_decode to end of function as suggested by Ricardo Ribalda
> - Link to v2: https://lore.kernel.org/r/20240221-uvc-host-video-decode-start-v2-1-88c6e17e487a@pengutronix.de
>
> Changes in v2:
> - Moved the EAGAIN bailout after the sequence handling as mentioned by Ricardo Ribalda
> - Link to v1: https://lore.kernel.org/r/20240221-uvc-host-video-decode-start-v1-1-228995925c70@pengutronix.de
> ---
>  drivers/media/usb/uvc/uvc_video.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 7cbf4692bd875..7471bff0ca894 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1078,9 +1078,6 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
>                         uvc_video_stats_update(stream);
>         }
>
> -       uvc_video_clock_decode(stream, buf, data, len);
> -       uvc_video_stats_decode(stream, data, len);
> -
>         /*
>          * Store the payload FID bit and return immediately when the buffer is
>          * NULL.
> @@ -1147,6 +1144,9 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
>                 return -EAGAIN;
>         }
>
> +       uvc_video_clock_decode(stream, buf, data, len);
> +       uvc_video_stats_decode(stream, data, len);
> +
>         stream->last_fid = fid;
>
>         return data[0];
>
> ---
> base-commit: d99e42ce6b8341d3f09e22c6706461ec900fe172
> change-id: 20240221-uvc-host-video-decode-start-af53df5924cd
>
> Best regards,
> --
> Michael Grzeschik <m.grzeschik@pengutronix.de>
>
>


-- 
Ricardo Ribalda