On 09/03/17 19:20, Anton Khirnov wrote:
> Quoting Mark Thompson (2017-03-05 00:57:47)
>> ---
>> configure | 5 +-
>> doc/APIchanges | 4 +
>> libavutil/Makefile | 2 +
>> libavutil/hwcontext.c | 4 +
>> libavutil/hwcontext.h | 1 +
>> libavutil/hwcontext_internal.h | 1 +
>> libavutil/hwcontext_opencl.c | 1182
>> ++++++++++++++++++++++++++++++++++++++++
>> libavutil/hwcontext_opencl.h | 117 ++++
>> libavutil/version.h | 2 +-
>> 9 files changed, 1316 insertions(+), 2 deletions(-)
>> create mode 100644 libavutil/hwcontext_opencl.c
>> create mode 100644 libavutil/hwcontext_opencl.h
>>
>> +static int opencl_frames_get_constraints(AVHWDeviceContext *hwdev,
>> + const void *hwconfig,
>> + AVHWFramesConstraints *constraints)
>> +{
>> + AVOpenCLDeviceContext *hwctx = hwdev->hwctx;
>> + cl_uint nb_image_formats;
>> + cl_image_format *image_formats = NULL;
>> + cl_int cle;
>> + enum AVPixelFormat pix_fmt;
>> + int err, pix_fmts_found;
>> + size_t max_width, max_height;
>> +
>> + cle = clGetDeviceInfo(hwctx->device_id, CL_DEVICE_IMAGE2D_MAX_WIDTH,
>> + sizeof(max_width), &max_width, NULL);
>> + if (cle != CL_SUCCESS) {
>> + av_log(hwdev, AV_LOG_ERROR, "Failed to query maximum "
>> + "supported image width: %d.\n", cle);
>> + } else {
>> + constraints->max_width = max_width;
>> + }
>> + cle = clGetDeviceInfo(hwctx->device_id, CL_DEVICE_IMAGE2D_MAX_HEIGHT,
>> + sizeof(max_height), &max_height, NULL);
>> + if (cle != CL_SUCCESS) {
>> + av_log(hwdev, AV_LOG_ERROR, "Failed to query maximum "
>> + "supported image height: %d.\n", cle);
>> + } else {
>> + constraints->max_height = max_height;
>> + }
>> + av_log(hwdev, AV_LOG_DEBUG, "Maximum supported image size %dx%d.\n",
>> + constraints->max_width, constraints->max_height);
>> +
>> + cle = clGetSupportedImageFormats(hwctx->context,
>> + CL_MEM_READ_WRITE,
>> + CL_MEM_OBJECT_IMAGE2D,
>> + 0, NULL, &nb_image_formats);
>> + if (cle != CL_SUCCESS) {
>> + av_log(hwdev, AV_LOG_ERROR, "Failed to query supported "
>> + "image formats: %d.\n", cle);
>> + err = AVERROR(ENOSYS);
>> + goto fail;
>> + }
>> + if (nb_image_formats == 0) {
>> + av_log(hwdev, AV_LOG_ERROR, "No image support in OpenCL "
>> + "driver (zero supported image formats).\n");
>> + err = AVERROR(ENOSYS);
>> + goto fail;
>> + }
>> +
>> + image_formats =
>> + av_malloc(nb_image_formats * sizeof(*image_formats));
>
> av_malloc_array?
Yep, ok.
>> + if (!image_formats) {
>> + err = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> +
>> + cle = clGetSupportedImageFormats(hwctx->context,
>> + CL_MEM_READ_WRITE,
>> + CL_MEM_OBJECT_IMAGE2D,
>> + nb_image_formats,
>> + image_formats, NULL);
>> + if (cle != CL_SUCCESS) {
>> + av_log(hwdev, AV_LOG_ERROR, "Failed to query supported "
>> + "image formats: %d.\n", cle);
>> + err = AVERROR(ENOSYS);
>> + goto fail;
>> + }
>> +
>> + pix_fmts_found = 0;
>> + for (pix_fmt = 0; pix_fmt < AV_PIX_FMT_NB; pix_fmt++) {
>> + cl_image_format image_format;
>> + cl_image_desc image_desc;
>> + int plane, i;
>> +
>> + for (plane = 0;; plane++) {
>> + err = opencl_get_plane_format(pix_fmt, plane, 0, 0,
>> + &image_format,
>> + &image_desc);
>
> Wouldn't it make sense to call this just once during frames context
> init?
Hmm, yeah, probably (with s/frames/device/). (This was significantly saner
before it did all the pixdesc stuff.)
>> + if (err < 0)
>> + break;
>> +
>> + for (i = 0; i < nb_image_formats; i++) {
>> + if (image_formats[i].image_channel_order ==
>> + image_format.image_channel_order &&
>> + image_formats[i].image_channel_data_type ==
>> + image_format.image_channel_data_type)
>> + break;
>> + }
>> + if (i == nb_image_formats) {
>> + err = AVERROR(EINVAL);
>> + break;
>> + }
>> + }
>> + if (err != AVERROR(ENOENT))
>> + continue;
>> +
>> + av_log(hwdev, AV_LOG_DEBUG, "Format %s supported.\n",
>> + av_get_pix_fmt_name(pix_fmt));
>> +
>> + constraints->valid_sw_formats =
>> + av_realloc_array(constraints->valid_sw_formats,
>> + pix_fmts_found + 2,
>> + sizeof(*constraints->valid_sw_formats));
>
> leaks on failure
> av_reallocp_array() perhaps?
Yeah.
>> +static int opencl_get_buffer(AVHWFramesContext *hwfc, AVFrame *frame)
>> +{
>> + AVOpenCLFrameDescriptor *desc;
>> + int plane;
>> +
>> + frame->buf[0] = av_buffer_pool_get(hwfc->pool);
>> + if (!frame->buf[0])
>> + return AVERROR(ENOMEM);
>> +
>> + desc = (AVOpenCLFrameDescriptor*)frame->buf[0]->data;
>> +
>> + for (plane = 0; plane < AV_NUM_DATA_POINTERS &&
>> + desc->image[plane]; plane++) {
>> + frame->data[plane] = (uint8_t*)desc->image[plane];
>> + frame->linesize[plane] = hwfc->width;
>
> This looks wrong, at least for subsampled formats. And what about
> alignment, is not needed?
The linesize is not really meaningful here at all now, I'll just remove it.
Image planes are just required to be aligned as a single element of the plane,
which must be aligned to the size of a single component multiplied by the
number of components rounded up to the next power of two. This is somewhat
weaker than the normal constraints in libav for SIMD.
>> + }
>> +
>> + frame->format = AV_PIX_FMT_OPENCL;
>> + frame->width = hwfc->width;
>> + frame->height = hwfc->height;
>> +
>> + return 0;
>> +}
>> +
>> +static int opencl_transfer_get_formats(AVHWFramesContext *hwfc,
>> + enum AVHWFrameTransferDirection dir,
>> + enum AVPixelFormat **formats)
>> +{
>> + enum AVPixelFormat *fmts;
>> +
>> + fmts = av_malloc_array(2, sizeof(*fmts));
>> + if (!fmts)
>> + return AVERROR(ENOMEM);
>> +
>> + fmts[0] = hwfc->sw_format;
>> + fmts[1] = AV_PIX_FMT_NONE;
>> +
>> + *formats = fmts;
>> + return 0;
>> +}
>> +
>> +static cl_command_queue opencl_get_command_queue(AVHWFramesContext *hwfc)
>> +{
>> + AVOpenCLFramesContext *fc = hwfc->hwctx;
>> + AVOpenCLDeviceContext *dc = hwfc->device_ctx->hwctx;
>> + OpenCLDeviceContext *ctx = hwfc->device_ctx->internal->priv;
>> +
>> + if (fc->command_queue)
>> + return fc->command_queue;
>> + if (dc->command_queue)
>> + return dc->command_queue;
>> + av_assert0(ctx->command_queue);
>> + return ctx->command_queue;
>> +}
>
> nit: perhaps it'd be more elegant to use clRetainCommandQueue to keep an
> internal reference and avoid this
Yeah, that would be better.
Thanks,
- Mark
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
