[FFmpeg-devel] [PATCH] avutil/opencl: add av_warn_unused_result

[Ganesh Ajjanagadde]

On Thu, Oct 15, 2015 at 11:17 PM, highgod0401 <highgod0401 at gmail.com> wrote:
>
> From: Ganesh Ajjanagadde
> Date: 2015-10-16 08:08
> To: ffmpeg-devel
> CC: Ganesh Ajjanagadde
> Subject: [FFmpeg-devel] [PATCH] avutil/opencl: add av_warn_unused_result
> This will trigger a few warnings. My config does not compile the opencl
> code, so this is untested.
>
> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
> ---
>  libavutil/opencl.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/libavutil/opencl.h b/libavutil/opencl.h
> index e423e55..6a5b0ab 100644
> --- a/libavutil/opencl.h
> +++ b/libavutil/opencl.h
> @@ -81,6 +81,7 @@ typedef struct {
>   *
>   * @return  >=0 on success, a negative error code in case of failure
>   */
> +av_warn_unused_result
>  int av_opencl_get_device_list(AVOpenCLDeviceList **device_list);
>
>  /**
> @@ -108,6 +109,7 @@ void av_opencl_free_device_list(AVOpenCLDeviceList
> **device_list);
>   * @return >=0 on success, a negative error code in case of failure
>   * @see av_opencl_get_option()
>   */
> +av_warn_unused_result
>  int av_opencl_set_option(const char *key, const char *val);
>
>  /**
> @@ -119,6 +121,7 @@ int av_opencl_set_option(const char *key, const char
> *val);
>   * @return  >=0 on success, a negative error code in case of failure
>   * @see av_opencl_set_option()
>   */
> +av_warn_unused_result
>  int av_opencl_get_option(const char *key, uint8_t **out_val);
>
>  /**
> @@ -161,6 +164,7 @@ const char *av_opencl_errstr(cl_int status);
>   * @param kernel_code    kernel code to be compiled in the OpenCL runtime
> environment
>   * @return  >=0 on success, a negative error code in case of failure
>   */
> +av_warn_unused_result
>  int av_opencl_register_kernel_code(const char *kernel_code);
>
>  /**
> @@ -170,6 +174,7 @@ int av_opencl_register_kernel_code(const char
> *kernel_code);
>   *                       application program, ignored if set to NULL
>   * @return >=0 on success, a negative error code in case of failure
>   */
> +av_warn_unused_result
>  int av_opencl_init(AVOpenCLExternalEnv *ext_opencl_env);
>
>  /**
> @@ -205,6 +210,7 @@ cl_command_queue av_opencl_get_command_queue(void);
>   * @param host_ptr     host pointer of the OpenCL buffer
>   * @return >=0 on success, a negative error code in case of failure
>   */
> +av_warn_unused_result
>  int av_opencl_buffer_create(cl_mem *cl_buf, size_t cl_buf_size, int flags,
> void *host_ptr);
>
>  /**
> @@ -215,6 +221,7 @@ int av_opencl_buffer_create(cl_mem *cl_buf, size_t
> cl_buf_size, int flags, void
>   * @param buf_size          size in bytes of the source and destination
> buffers
>   * @return >=0 on success, a negative error code in case of failure
>   */
> +av_warn_unused_result
>  int av_opencl_buffer_write(cl_mem dst_cl_buf, uint8_t *src_buf, size_t
> buf_size);
>
>  /**
> @@ -225,6 +232,7 @@ int av_opencl_buffer_write(cl_mem dst_cl_buf, uint8_t
> *src_buf, size_t buf_size)
>   * @param buf_size          size in bytes of the source and destination
> buffers
>   * @return >=0 on success, a negative error code in case of failure
>   */
> +av_warn_unused_result
>  int av_opencl_buffer_read(uint8_t *dst_buf, cl_mem src_cl_buf, size_t
> buf_size);
>
>  /**
> @@ -240,6 +248,7 @@ int av_opencl_buffer_read(uint8_t *dst_buf, cl_mem
> src_cl_buf, size_t buf_size);
>   * @param src_plane_num      number of source image planes
>   * @return >=0 on success, a negative error code in case of failure
>   */
> +av_warn_unused_result
>  int av_opencl_buffer_write_image(cl_mem dst_cl_buf, size_t cl_buffer_size,
> int dst_cl_offset,
>                                   uint8_t **src_data, int *plane_size, int
> plane_num);
>
> @@ -253,6 +262,7 @@ int av_opencl_buffer_write_image(cl_mem dst_cl_buf,
> size_t cl_buffer_size, int d
>   * @param src_cl_buf_size    size in bytes of OpenCL buffer
>   * @return >=0 on success, a negative error code in case of failure
>   */
> +av_warn_unused_result
>  int av_opencl_buffer_read_image(uint8_t **dst_data, int *plane_size, int
> plane_num,
>                                  cl_mem src_cl_buf, size_t cl_buffer_size);
>
> --
> 2.6.1
>
> Hi
>
> Could I ask a question? Why should we add the av_warn_unused_result?

It is not an absolute necessity, but highly recommended. Basically,
functions which can fail and propagate error codes need their error
codes propagated. See the original commit:
1d4af04adf99301e21d7364d72f5570f5219083a for its rationale. It already
has caught quite a few bugs (some of them CIDs), with no introduction
of false positives yet. As long as one judiciously applies it (and not
just everywhere), it should be very helpful.

Note that it not only helps FFmpeg, but also users of FFmpeg who use
our headers: if they do not check the return codes from our API's,
they will get warnings. By the way, currently it is for GCC, etc -
maybe there is an equivalent for MSVC etc that someone may add in the
future (I personally don't care).

I looked at the OpenCL stuff - seems to me like OpenCL can error out
on this stuff, and the FFmpeg code here does return associated
AVERROR's. These need to be propagated to whoever uses the API.

>
> Thanks
> Best regards
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel