[FFmpeg-devel] [PATCH v2] libavutil/hwcontext_opencl.c: fix bug in `opencl_get_plane_format`
Mark Thompson
sw at jkqxz.net
Mon Apr 8 22:48:38 EEST 2019
On 08/04/2019 03:01, Jarek Samic wrote:
> The `opencl_get_plane_format` function was incorrectly determining the
> value used to set the image channel order. This resulted in all RGB
> pixel formats being set to the `CL_RGBA` pixel format, regardless of
> whether or not they actually *were* RGBA.
>
> This patch fixes the issue by using the `offset` and depth of components
> rather than the loop index to determine the value of `order`.
>
> Signed-off-by: Jarek Samic <cldfire3 at gmail.com>
> ---
> I have updated this patch in response to the comments on the first version.
> RGB is no longer special-cased, the 2, 3, and 4 mappings to `CL_R` have been
> removed, and the mapping for `CL_ARGB` has been changed to the correct value.
>
> libavutil/hwcontext_opencl.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c
> index b116c5b708..593de1ca41 100644
> --- a/libavutil/hwcontext_opencl.c
> +++ b/libavutil/hwcontext_opencl.c
> @@ -1419,8 +1419,9 @@ static int opencl_get_plane_format(enum AVPixelFormat pixfmt,
> // from the same component.
> if (step && comp->step != step)
> return AVERROR(EINVAL);
> - order = order * 10 + c + 1;
> +
> depth = comp->depth;
> + order = order * 10 + comp->offset / ((depth + 7) / 8) + 1;
> step = comp->step;
> alpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA &&
> c == desc->nb_components - 1);
This part LGTM, I can split it off and apply it on its own if you like.
> @@ -1456,14 +1457,11 @@ static int opencl_get_plane_format(enum AVPixelFormat pixfmt,
> case order: image_format->image_channel_order = type; break;
> switch (order) {
> CHANNEL_ORDER(1, CL_R);
> - CHANNEL_ORDER(2, CL_R);
> - CHANNEL_ORDER(3, CL_R);
> - CHANNEL_ORDER(4, CL_R);
> CHANNEL_ORDER(12, CL_RG);
> CHANNEL_ORDER(23, CL_RG);
23 should be gone too, I think?
> CHANNEL_ORDER(1234, CL_RGBA);
> + CHANNEL_ORDER(2341, CL_ARGB);
> CHANNEL_ORDER(3214, CL_BGRA);
> - CHANNEL_ORDER(4123, CL_ARGB);
I'm not sure I believe this part:
1 = R
2 = G
3 = B
4 = A
gives
RGBA -> 1234
BGRA -> 3214
ARGB -> 4123
ABGR -> 4321
The others match, so why would ARGB be different? 2341 should be GBAR.
(Can you try this with multiple ARGB sources or OpenCL ICDs? Maybe there is a bug somewhere else...)
> #ifdef CL_ABGR
> CHANNEL_ORDER(4321, CL_ABGR);
> #endif
>
Thanks,
- Mark
More information about the ffmpeg-devel
mailing list