On Sat, Feb 23, 2013 at 02:54:09PM -0500, Derek Buitenhuis wrote:
> Signed-off-by: Derek Buitenhuis <[email protected]>
> ---
> 
> The change between v3 and v4 is the check for the src and dst formats.
> 
> ---
>  libswscale/swscale_unscaled.c |   85 
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
> 
> diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
> index 78090f1..c754c65 100644
> --- a/libswscale/swscale_unscaled.c
> +++ b/libswscale/swscale_unscaled.c
> @@ -468,6 +468,87 @@ static int planarRgbToRgbWrapper(SwsContext *c, const 
> uint8_t *src[],
>      return srcSliceH;
>  }
>  
> +static void packedtogbr24p(const uint8_t *src, int srcStride,
> +                             uint8_t *dst[], int dstStride[], int srcSliceH,
> +                             int alpha_first, int inc_size, int width)

diego-valign-nit

> +{
> +    uint8_t *dest[3];
> +    int x, h;
> +
> +    dest[0] = dst[0];
> +    dest[1] = dst[1];
> +    dest[2] = dst[2];
> +
> +    if (alpha_first)
> +        src++;
> +
> +    for (h = 0; h < srcSliceH; h++) {
> +        for (x = 0; x < width; x++) {
> +            dest[0][x] = src[0];
> +            dest[1][x] = src[1];
> +            dest[2][x] = src[2];
> +
> +            src += inc_size;
> +        }
> +        src     += srcStride - width * inc_size;
> +        dest[0] += dstStride[0];
> +        dest[1] += dstStride[1];
> +        dest[2] += dstStride[2];
> +    }
> +}
> +
> +static int rgbToPlanarRgbWrapper(SwsContext *c, const uint8_t *src[],
> +                                 int srcStride[], int srcSliceY, int 
> srcSliceH,
> +                                 uint8_t *dst[], int dstStride[])
> +{
> +    int alpha_first = 0;
> +    int stride102[] = { dstStride[1], dstStride[0], dstStride[2] };
> +    int stride201[] = { dstStride[2], dstStride[0], dstStride[1] };
> +    uint8_t *dst102[] = { dst[1] + srcSliceY * dstStride[1],
> +                          dst[0] + srcSliceY * dstStride[0],
> +                          dst[2] + srcSliceY * dstStride[2] };
> +    uint8_t *dst201[] = { dst[2] + srcSliceY * dstStride[2],
> +                          dst[0] + srcSliceY * dstStride[0],
> +                          dst[1] + srcSliceY * dstStride[1] };
> +
> +    if (c->dstFormat != PIX_FMT_GBRP) {
> +        av_log(c, AV_LOG_ERROR, "unsupported planar RGB conversion %s -> 
> %s\n",
> +               av_get_pix_fmt_name(c->srcFormat),
> +               av_get_pix_fmt_name(c->dstFormat));
> +        return srcSliceH;
> +    }

looks pointless

> +
> +    switch (c->srcFormat) {
> +    case PIX_FMT_RGB24:
> +        packedtogbr24p((const uint8_t *) src[0], srcStride[0], dst201,
> +                       stride201, srcSliceH, alpha_first, 3, c->srcW);
> +        break;
> +    case PIX_FMT_BGR24:
> +        packedtogbr24p((const uint8_t *) src[0], srcStride[0], dst102,
> +                       stride102, srcSliceH, alpha_first, 3, c->srcW);
> +        break;
> +    case PIX_FMT_ARGB:
> +        alpha_first = 1;
> +    case PIX_FMT_RGBA:
> +        packedtogbr24p((const uint8_t *) src[0], srcStride[0], dst201,
> +                       stride201, srcSliceH, alpha_first, 4, c->srcW);
> +        break;
> +    case PIX_FMT_ABGR:
> +        alpha_first = 1;
> +    case PIX_FMT_BGRA:
> +        packedtogbr24p((const uint8_t *) src[0], srcStride[0], dst102,
> +                       stride102, srcSliceH, alpha_first, 4, c->srcW);

is it just me or the casts looks unneeded?

Also IMO it can be simplified even further and only one call left.
The only slightly nontrivial step is selecting dst102+stride102 or
dst201+stride201. And probably it can even be extended to handle your
favourite raw 16 bit per component RGB. But that's probably just creeping
featurism.

> +        break;
> +    default:
> +        av_log(c, AV_LOG_ERROR,
> +               "unsupported planar RGB conversion %s -> %s\n",
> +               av_get_pix_fmt_name(c->srcFormat),
> +               av_get_pix_fmt_name(c->dstFormat));
> +    }

should be caught at init IMO

> +    return srcSliceH;
> +}
> +
>  #define isRGBA32(x) (            \
>             (x) == AV_PIX_FMT_ARGB   \
>          || (x) == AV_PIX_FMT_RGBA   \
> @@ -943,6 +1024,10 @@ void ff_get_unscaled_swscale(SwsContext *c)
>      if (srcFormat == AV_PIX_FMT_GBRP && isPlanar(srcFormat) && 
> isByteRGB(dstFormat))
>          c->swScale = planarRgbToRgbWrapper;
>  
> +    if (av_pix_fmt_descriptors[srcFormat].comp[0].depth_minus1 == 7 &&
> +        isPackedRGB(srcFormat) && dstFormat == AV_PIX_FMT_GBRP)
> +        c->swScale = rgbToPlanarRgbWrapper;
> +
>      /* bswap 16 bits per pixel/component packed formats */
>      if (IS_DIFFERENT_ENDIANESS(srcFormat, dstFormat, AV_PIX_FMT_BGR444) ||
>          IS_DIFFERENT_ENDIANESS(srcFormat, dstFormat, AV_PIX_FMT_BGR48)  ||
> -- 

In general probably OK.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to