All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
To: Ezequiel Garcia <ezequiel-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Cc: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>,
	Alexandre Courbot
	<acourbot-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	Jonas Karlman <jonas-uIzNG4q0ceqzQB+pC5nmwQ@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Jeffrey Kardatzke
	<jkardatzke-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	kernel-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org,
	linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v2 2/3] media: uapi: Add VP9 stateless decoder controls
Date: Sat, 2 May 2020 20:37:07 +0200	[thread overview]
Message-ID: <20200502203707.402ea3cd@collabora.com> (raw)
In-Reply-To: <bf475e70cca6f9ebf645aed51276e57668eaf43b.camel-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

On Fri, 01 May 2020 13:57:49 -0300
Ezequiel Garcia <ezequiel-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> wrote:

> > > +
> > > +.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
> > > +
> > > +.. flat-table:: enum v4l2_vp9_reset_frame_context
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +    :widths:       1 2
> > > +
> > > +    * - ``V4L2_VP9_RESET_FRAME_CTX_NONE``
> > > +      - Do not reset any frame context.
> > > +    * - ``V4L2_VP9_RESET_FRAME_CTX_NONE_ALT``
> > > +      - Do not reset any frame context. This is an alternative value for
> > > +        V4L2_VP9_RESET_FRAME_CTX_NONE.  
> > 
> > Add `` around V4L2_VP9_RESET_FRAME_CTX_NONE.
> >   
> 
> Hm, now that I look closer, what's the point
> of having the NONE_ALT in our uAPI if it
> has same meaning as NONE?
> 
> I think it can be removed.

The intent was to match the spec so that one can pass the value
extracted from the bitstream directly.

> > 
> > I got several smatch warnings:
> > 
> > smatch: ERRORS
> > drivers/media/v4l2-core/v4l2-ctrls.c:1880 validate_vp9_frame_decode_params() warn: was && intended here instead of ||?
> > 
> > (Commented on this ^^^ one above)
> > 
> > drivers/staging/media/rkvdec/rkvdec-vp9.c:426 init_intra_only_probs() error: buffer overflow 'ptr' 9 <= 69
> > drivers/staging/media/rkvdec/rkvdec-vp9.c:1478 rkvdec_vp9_done() error: potentially dereferencing uninitialized 'ctrl'.
> > drivers/staging/media/rkvdec/rkvdec-vp9.c:1483 rkvdec_vp9_done() error: uninitialized symbol 'dec_dst_buf'.
> > drivers/staging/media/rkvdec/rkvdec-vp9.c:941:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
> > drivers/staging/media/rkvdec/rkvdec-vp9.c:1466:40: warning: variable 'fctx' set but not used [-Wunused-but-set-variable]
> >   
> 
> Oh, I'll run smatch and fix them all.

Oops!

WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Ezequiel Garcia <ezequiel@collabora.com>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
	linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org,
	linux-kernel@vger.kernel.org, Tomasz Figa <tfiga@chromium.org>,
	kernel@collabora.com, Jonas Karlman <jonas@kwiboo.se>,
	Heiko Stuebner <heiko@sntech.de>,
	Alexandre Courbot <acourbot@chromium.org>,
	Jeffrey Kardatzke <jkardatzke@chromium.org>
Subject: Re: [PATCH v2 2/3] media: uapi: Add VP9 stateless decoder controls
Date: Sat, 2 May 2020 20:37:07 +0200	[thread overview]
Message-ID: <20200502203707.402ea3cd@collabora.com> (raw)
In-Reply-To: <bf475e70cca6f9ebf645aed51276e57668eaf43b.camel@collabora.com>

On Fri, 01 May 2020 13:57:49 -0300
Ezequiel Garcia <ezequiel@collabora.com> wrote:

> > > +
> > > +.. tabularcolumns:: |p{1.5cm}|p{6.3cm}|p{9.4cm}|
> > > +
> > > +.. flat-table:: enum v4l2_vp9_reset_frame_context
> > > +    :header-rows:  0
> > > +    :stub-columns: 0
> > > +    :widths:       1 2
> > > +
> > > +    * - ``V4L2_VP9_RESET_FRAME_CTX_NONE``
> > > +      - Do not reset any frame context.
> > > +    * - ``V4L2_VP9_RESET_FRAME_CTX_NONE_ALT``
> > > +      - Do not reset any frame context. This is an alternative value for
> > > +        V4L2_VP9_RESET_FRAME_CTX_NONE.  
> > 
> > Add `` around V4L2_VP9_RESET_FRAME_CTX_NONE.
> >   
> 
> Hm, now that I look closer, what's the point
> of having the NONE_ALT in our uAPI if it
> has same meaning as NONE?
> 
> I think it can be removed.

The intent was to match the spec so that one can pass the value
extracted from the bitstream directly.

> > 
> > I got several smatch warnings:
> > 
> > smatch: ERRORS
> > drivers/media/v4l2-core/v4l2-ctrls.c:1880 validate_vp9_frame_decode_params() warn: was && intended here instead of ||?
> > 
> > (Commented on this ^^^ one above)
> > 
> > drivers/staging/media/rkvdec/rkvdec-vp9.c:426 init_intra_only_probs() error: buffer overflow 'ptr' 9 <= 69
> > drivers/staging/media/rkvdec/rkvdec-vp9.c:1478 rkvdec_vp9_done() error: potentially dereferencing uninitialized 'ctrl'.
> > drivers/staging/media/rkvdec/rkvdec-vp9.c:1483 rkvdec_vp9_done() error: uninitialized symbol 'dec_dst_buf'.
> > drivers/staging/media/rkvdec/rkvdec-vp9.c:941:6: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
> > drivers/staging/media/rkvdec/rkvdec-vp9.c:1466:40: warning: variable 'fctx' set but not used [-Wunused-but-set-variable]
> >   
> 
> Oh, I'll run smatch and fix them all.

Oops!

  parent reply	other threads:[~2020-05-02 18:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-10 11:51 [PATCH v2 0/3] media: rkvdec: Add a VP9 backend Ezequiel Garcia
2020-04-10 11:51 ` [PATCH v2 1/3] media: v4l2-ctrls: Add the [__]v4l2_ctrl_s_ctrl_compound() helpers Ezequiel Garcia
2020-04-10 11:51 ` [PATCH v2 2/3] media: uapi: Add VP9 stateless decoder controls Ezequiel Garcia
     [not found]   ` <20200410115113.31728-3-ezequiel-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
2020-04-16 11:47     ` Hans Verkuil
2020-04-16 11:47       ` Hans Verkuil
2020-05-01 16:57       ` Ezequiel Garcia
     [not found]         ` <bf475e70cca6f9ebf645aed51276e57668eaf43b.camel-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
2020-05-02 18:37           ` Boris Brezillon [this message]
2020-05-02 18:37             ` Boris Brezillon
2020-05-02 22:55             ` Ezequiel Garcia
2020-05-04 18:01               ` Nicolas Dufresne
     [not found]                 ` <e53824aed3eeb27419e5399576cce028f0ba8203.camel-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
2020-05-04 18:38                   ` Nicolas Dufresne
2020-05-04 18:38                     ` Nicolas Dufresne
2020-05-04 19:13                     ` Boris Brezillon
2020-05-04 19:06                 ` Boris Brezillon
2020-04-10 11:51 ` [PATCH v2 3/3] media: rkvdec: Add the VP9 backend Ezequiel Garcia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200502203707.402ea3cd@collabora.com \
    --to=boris.brezillon-zgy8ohtn/8qb+jhodadfcq@public.gmane.org \
    --cc=acourbot-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=ezequiel-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org \
    --cc=heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org \
    --cc=hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org \
    --cc=jkardatzke-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=jonas-uIzNG4q0ceqzQB+pC5nmwQ@public.gmane.org \
    --cc=kernel-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.