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 <cldfi...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".