On Tue, 21 Feb 2017 22:43:05 +0000
Mark Thompson <[email protected]> wrote:

> On 21/02/17 14:21, wm4 wrote:
> > On Tue, 21 Feb 2017 13:18:55 +0000
> > Mark Thompson <[email protected]> wrote:
> >   
> >> On 21/02/17 08:00, wm4 wrote:  
> >>> On Mon, 20 Feb 2017 23:06:21 +0000
> >>> Mark Thompson <[email protected]> wrote:
> >>>     
> >>>>>> +#ifdef CL_ABGR
> >>>>>> +        if (0)      
> >>>>>
> >>>>> wut      
> >>>>
> >>>> CL_ABGR didn't exist in OpenCL 1.2, which is really the target.  It's 
> >>>> present in later versions, though, so it's included here if possible for 
> >>>> symmetry.    
> >>>
> >>> I meant what do the "if (0)"s do here? They're highly confusing at
> >>> best.    
> >>
> >> Avoiding more gotos?  This function should really be a table, but it's 
> >> just tricky enough to make that annoying.  I'll see if I can improve it.  
> > 
> > Yeah, I think that would be better. Also you could say you're using
> > switch-case as some sort of computed goto, which it really is, but ugly
> > and hard to read.  
> 
> Right, taking this function to its (perhaps il)logical conclusion I can 
> replace it with:
> 
> static int opencl_get_plane_format(enum AVPixelFormat pixfmt,
>                                    int plane, int width, int height,
>                                    cl_image_format *image_format,
>                                    cl_image_desc *image_desc)
> {
>     const AVPixFmtDescriptor *desc;
>     const AVComponentDescriptor *comp;
>     int channels = 0, order = 0, depth = 0, step = 0;
>     int wsub, hsub, alpha;
>     int c;
> 
>     desc = av_pix_fmt_desc_get(pixfmt);
> 
>     // Only normal images are allowed.
>     if (desc->flags & (AV_PIX_FMT_FLAG_BITSTREAM |
>                        AV_PIX_FMT_FLAG_HWACCEL   |
>                        AV_PIX_FMT_FLAG_PAL))
>         return AVERROR(EINVAL);
> 
>     wsub = 1 << desc->log2_chroma_w;
>     hsub = 1 << desc->log2_chroma_h;
>     // Subsampled components must be exact.
>     if (width & wsub - 1 || height & hsub - 1)
>         return AVERROR(EINVAL);
> 
>     for (c = 0; c < desc->nb_components; c++) {
>         comp = &desc->comp[c];
>         if (comp->plane != plane)
>             continue;
>         // The step size must be a power of two.
>         if (comp->step != 1 && comp->step != 2 &&
>             comp->step != 4 && comp->step != 8)
>             return AVERROR(EINVAL);
>         // The bits in each component must be packed in the
>         // most-significant-bits of the relevant bytes.
>         if (comp->shift + comp->depth != 8 &&
>             comp->shift + comp->depth != 16)
>             return AVERROR(EINVAL);
>         // The depth must not vary between components.
>         if (depth && comp->depth != depth)
>             return AVERROR(EINVAL);
>         // If a single data element crosses multiple bytes then
>         // it must match the native endianness.
>         if (comp->depth > 8 &&
>             HAVE_BIGENDIAN == !(desc->flags & AV_PIX_FMT_FLAG_BE))
>             return AVERROR(EINVAL);
>         // A single data element must not contain multiple samples
>         // from the same component.
>         if (step && comp->step != step)
>             return AVERROR(EINVAL);
>         order = order * 10 + c + 1;
>         depth = comp->depth;
>         step  = comp->step;
>         alpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA &&
>                  c == desc->nb_components - 1);
>         ++channels;
>     }
>     if (channels == 0)
>         return AVERROR(ENOENT);
> 
>     memset(image_format, 0, sizeof(*image_format));
>     memset(image_desc,   0, sizeof(*image_desc));
>     image_desc->image_type = CL_MEM_OBJECT_IMAGE2D;
> 
>     if (plane == 0 || alpha) {
>         image_desc->image_width     = width;
>         image_desc->image_height    = height;
>         image_desc->image_row_pitch = step * width;
>     } else {
>         image_desc->image_width     = width  / wsub;
>         image_desc->image_height    = height / hsub;
>         image_desc->image_row_pitch = step * width / wsub;
>     }
> 
>     if (depth <= 8) {
>         image_format->image_channel_data_type = CL_UNORM_INT8;
>     } else {
>         if (depth <= 16)
>             image_format->image_channel_data_type = CL_UNORM_INT16;
>         else
>             return AVERROR(EINVAL);
>     }
> 
> #define CHANNEL_ORDER(order, type) \
>     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);
>         CHANNEL_ORDER(1234, CL_RGBA);
>         CHANNEL_ORDER(3214, CL_BGRA);
>         CHANNEL_ORDER(4123, CL_ARGB);
> #ifdef CL_ABGR
>         CHANNEL_ORDER(4321, CL_ABGR);
> #endif
>     default:
>         return AVERROR(EINVAL);
>     }
> #undef CHANNEL_ORDER
> 
>     return 0;
> }
> 
> ...which gives the supported formats of Beignet as:
> 
> yuv420p
> yuv422p
> yuv444p
> yuv410p
> yuv411p
> gray
> yuvj420p
> yuvj422p
> yuvj444p
> nv12
> nv21
> argb
> rgba
> abgr
> bgra
> gray16le
> yuv440p
> yuvj440p
> yuva420p
> yuv420p16le
> yuv422p16le
> yuv444p16le
> ya8
> gbrp
> gbrp16le
> yuva422p
> yuva444p
> yuva420p16le
> yuva422p16le
> yuva444p16le
> nv16
> rgba64le
> bgra64le
> ya16le
> gbrap
> gbrap16le
> p010le
> 
> :)

Nice. And yes, dealing with pixdesc can be pretty annoying. But it
looks like you figured it out.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to