[FFmpeg-devel] [PATCH] acvodec/lipopenjpeg: Improve XYZ color space detection

[Michael Bradshaw]

On Mon, Mar 2, 2015 at 7:08 AM, Vilius Grigaliūnas <
vilius.grigaliunas at gmail.com> wrote:

> Input files in XYZ color space are incorrecly detected as RGB which results
> in incorrect output colors.
>
> This changes pixel format detection to try XYZ before RGB when
> color space provided by libopenjepg is unknown.
>

Is this really desirable, though? The most common case should be the
default. If XYZ is the most common use case for ffmpeg/avcodec users, then
sure, this is good. But I'm not convinced XYZ is more common than RGB. I
believe RGB is more common than XYZ, and as such should remain the default.


> ---
>  libavcodec/libopenjpegdec.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c
> index 02b1ceb..23febd7 100644
> --- a/libavcodec/libopenjpegdec.c
> +++ b/libavcodec/libopenjpegdec.c
> @@ -77,7 +77,7 @@ static const enum AVPixelFormat
> libopenjpeg_yuv_pix_fmts[]  = {
>      YUV_PIXEL_FORMATS
>  };
>  static const enum AVPixelFormat libopenjpeg_all_pix_fmts[]  = {
> -    RGB_PIXEL_FORMATS, GRAY_PIXEL_FORMATS, YUV_PIXEL_FORMATS,
> XYZ_PIXEL_FORMATS
> +    XYZ_PIXEL_FORMATS, RGB_PIXEL_FORMATS, GRAY_PIXEL_FORMATS,
> YUV_PIXEL_FORMATS
>  };
>
>  typedef struct LibOpenJPEGContext {
> @@ -184,10 +184,11 @@ static inline void
> libopenjpeg_copy_to_packed8(AVFrame *picture, opj_image_t *im
>
>  static inline void libopenjpeg_copy_to_packed16(AVFrame *picture,
> opj_image_t *image) {
>      uint16_t *img_ptr;
> +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(picture->format);
>      int index, x, y, c;
>      int adjust[4];
>      for (x = 0; x < image->numcomps; x++)
> -        adjust[x] =
> FFMAX(FFMIN(av_pix_fmt_desc_get(picture->format)->comp[x].depth_minus1 + 1
> - image->comps[x].prec, 8), 0);
> +        adjust[x] = FFMAX(FFMIN(desc->comp[x].depth_minus1 + 1 -
> image->comps[x].prec, 8), 0);
>

This small cleanup/micro optimization is better in a different patch. It
seems unrelated to testing XYZ before RGB. Same for the similar change to
libopenjpeg_copyto16().


>
>      for (y = 0; y < picture->height; y++) {
>          index   = y * picture->width;
> @@ -195,7 +196,7 @@ static inline void
> libopenjpeg_copy_to_packed16(AVFrame *picture, opj_image_t *i
>          for (x = 0; x < picture->width; x++, index++)
>              for (c = 0; c < image->numcomps; c++)
>                  *img_ptr++ = (1 << image->comps[c].prec - 1) *
> image->comps[c].sgnd +
> -                             (unsigned)image->comps[c].data[index] <<
> adjust[c];
> +                             (unsigned)image->comps[c].data[index] <<
> adjust[c] << desc->comp[c].shift;
>

This seems like an unrelated change to XYZ detection as well.


>      }
>  }
>
> @@ -220,10 +221,11 @@ static inline void libopenjpeg_copyto8(AVFrame
> *picture, opj_image_t *image) {
>  static inline void libopenjpeg_copyto16(AVFrame *picture, opj_image_t
> *image) {
>      int *comp_data;
>      uint16_t *img_ptr;
> +    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(picture->format);
>      int index, x, y;
>      int adjust[4];
>      for (x = 0; x < image->numcomps; x++)
> -        adjust[x] =
> FFMAX(FFMIN(av_pix_fmt_desc_get(picture->format)->comp[x].depth_minus1 + 1
> - image->comps[x].prec, 8), 0);
> +        adjust[x] = FFMAX(FFMIN(desc->comp[x].depth_minus1 + 1 -
> image->comps[x].prec, 8), 0);
>
>      for (index = 0; index < image->numcomps; index++) {
>          comp_data = image->comps[index].data;
> @@ -231,7 +233,7 @@ static inline void libopenjpeg_copyto16(AVFrame
> *picture, opj_image_t *image) {
>              img_ptr = (uint16_t *)(picture->data[index] + y *
> picture->linesize[index]);
>              for (x = 0; x < image->comps[index].w; x++) {
>                  *img_ptr = (1 << image->comps[index].prec - 1) *
> image->comps[index].sgnd +
> -                           (unsigned)*comp_data << adjust[index];
> +                           (unsigned)*comp_data << adjust[index] <<
> desc->comp[index].shift;
>                  img_ptr++;
>                  comp_data++;
>              }
> --
> 1.7.9.5
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>