[vlc-devel] [PATCH v2] codec: jpeg encoder implemented

Rafaël Carré funman at videolan.org
Tue Jan 28 11:21:54 CET 2014


Hello,

I had a look at the decoder because I did not understand error handling.
My patch set applies to vlc.git.

> This patch implements jpeg encoder using libjpeg.
> 
> ---
>  modules/codec/jpeg.c | 211 +++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 197 insertions(+), 14 deletions(-)
> 
> diff --git a/modules/codec/jpeg.c b/modules/codec/jpeg.c
> index 32964cc..54bf75c 100644
> --- a/modules/codec/jpeg.c
> +++ b/modules/codec/jpeg.c
> @@ -1,7 +1,7 @@
>  /*****************************************************************************
>   * jpeg.c: jpeg decoder module making use of libjpeg.
>   *****************************************************************************
> - * Copyright (C) 2013 VLC authors and VideoLAN
> + * Copyright (C) 2013-2014 VLC authors and VideoLAN
>   *
>   * Authors: Maxim Bublis <b at codemonkey.ru>
>   *
> @@ -30,18 +30,32 @@
>  #include <jpeglib.h>
>  #include <setjmp.h>
>  
> +/* JPEG_SYS_COMMON_MEMBERS:
> + * members common to encoder and decoder descriptors
> + */
> +#define JPEG_SYS_COMMON_MEMBERS                             \
> +/**@{*/                                                     \
> +    /* libjpeg error handler manager */                     \
> +    struct jpeg_error_mgr err;                              \
> +                                                            \
> +    /* setjmp buffer for internal libjpeg error handling */ \
> +    jmp_buf setjmp_buffer;                                  \
> +                                                            \
> +    vlc_object_t *p_obj;                                    \
> +                                                            \
> +/**@}*/                                                     \
> +
> +#define ENC_CFG_PREFIX "sout-jpeg-"
> +#define ENC_QUALITY_TEXT N_("Quality level")
> +#define ENC_QUALITY_LONGTEXT N_("Quality level " \
> +    "for encoding (this can enlarge or reduce output image size).")
> +
>  /*
>   * jpeg decoder descriptor
>   */
>  struct decoder_sys_t
>  {
> -    /* libjpeg error handler manager */
> -    struct jpeg_error_mgr err;
> -
> -    /* setjmp buffer for internal libjpeg error handling */
> -    jmp_buf setjmp_buffer;
> -
> -    decoder_t *p_dec;
> +    JPEG_SYS_COMMON_MEMBERS
>  };
>  
>  static int  OpenDecoder(vlc_object_t *);
> @@ -50,15 +64,41 @@ static void CloseDecoder(vlc_object_t *);
>  static picture_t *DecodeBlock(decoder_t *, block_t **);
>  
>  /*
> + * jpeg encoder descriptor
> + */
> +struct encoder_sys_t
> +{
> +    JPEG_SYS_COMMON_MEMBERS
> +
> +    int i_quality;
> +};
> +
> +static int  OpenEncoder(vlc_object_t *);
> +static void CloseEncoder(vlc_object_t *);
> +
> +static block_t *EncodeBlock(encoder_t *, picture_t *);
> +
> +/*
>   * Module descriptor
>   */
>  vlc_module_begin()
>      set_category(CAT_INPUT)
>      set_subcategory(SUBCAT_INPUT_VCODEC)
> +    /* decoder main module */
>      set_description(N_("JPEG image decoder"))
>      set_capability("decoder", 1000)
>      set_callbacks(OpenDecoder, CloseDecoder)
>      add_shortcut("jpeg")
> +
> +    /* encoder submodule */
> +    add_submodule()
> +    add_shortcut("jpeg")
> +    set_section(N_("Encoding"), NULL)
> +    set_description(N_("JPEG image encoder"))
> +    set_capability("encoder", 1000)
> +    set_callbacks(OpenEncoder, CloseEncoder)
> +    add_integer_with_range(ENC_CFG_PREFIX "quality", 95, 0, 100,
> +                           ENC_QUALITY_TEXT, ENC_QUALITY_LONGTEXT, true)
>  vlc_module_end()
>  
>  /*
> @@ -80,7 +120,7 @@ static int OpenDecoder(vlc_object_t *p_this)
>          return VLC_ENOMEM;
>      }
>  
> -    p_dec->p_sys->p_dec = p_dec;
> +    p_dec->p_sys->p_obj = p_this;
>  
>      /* Set output properties */
>      p_dec->fmt_out.i_cat = VIDEO_ES;
> @@ -94,7 +134,7 @@ static int OpenDecoder(vlc_object_t *p_this)
>  /*
>   * Exit error handler for libjpeg
>   */
> -static void user_error_exit(j_common_ptr p_jpeg)
> +static void decoder_error_exit(j_common_ptr p_jpeg)

No need to define 2 different callbacks that do the same thing.

>  {
>      decoder_sys_t *p_sys = (decoder_sys_t *)p_jpeg->err;
>      p_sys->err.output_message(p_jpeg);
> @@ -104,13 +144,13 @@ static void user_error_exit(j_common_ptr p_jpeg)
>  /*
>   * Emit message error handler for libjpeg
>   */
> -static void user_error_message(j_common_ptr p_jpeg)
> +static void decoder_error_message(j_common_ptr p_jpeg)
>  {
>      char error_msg[JMSG_LENGTH_MAX];
>  
>      decoder_sys_t *p_sys = (decoder_sys_t *)p_jpeg->err;
>      p_sys->err.format_message(p_jpeg, error_msg);
> -    msg_Err(p_sys->p_dec, "%s", error_msg);
> +    msg_Err(p_sys->p_obj, "%s", error_msg);
>  }
>  
>  /*
> @@ -140,8 +180,8 @@ static picture_t *DecodeBlock(decoder_t *p_dec, block_t **pp_block)
>      }
>  
>      p_jpeg.err = jpeg_std_error(&p_sys->err);
> -    p_sys->err.error_exit = user_error_exit;
> -    p_sys->err.output_message = user_error_message;
> +    p_sys->err.error_exit = decoder_error_exit;
> +    p_sys->err.output_message = decoder_error_message;
>  
>      /* libjpeg longjmp's there in case of error */
>      if (setjmp(p_sys->setjmp_buffer))
> @@ -222,3 +262,146 @@ static void CloseDecoder(vlc_object_t *p_this)
>  
>      free(p_sys);
>  }
> +
> +/*
> + * Probe the encoder and return score
> + */
> +static int OpenEncoder(vlc_object_t *p_this)
> +{
> +    encoder_t *p_enc = (encoder_t *)p_this;
> +
> +    if (p_enc->fmt_out.i_codec != VLC_CODEC_JPEG)
> +    {
> +        return VLC_EGENERIC;
> +    }
> +
> +    /* Allocate the memory needed to store encoder's structure */
> +    p_enc->p_sys = malloc(sizeof(encoder_sys_t));
> +    if (p_enc->p_sys == NULL)
> +    {
> +        return VLC_ENOMEM;
> +    }
> +
> +    p_enc->p_sys->p_obj = p_this;
> +
> +    p_enc->pf_encode_video = EncodeBlock;
> +
> +    p_enc->p_sys->i_quality = var_GetInteger(p_enc, ENC_CFG_PREFIX "quality");
> +
> +    return VLC_SUCCESS;
> +}
> +
> +/*
> + * Exit error handler for libjpeg
> + */
> +static void encoder_error_exit(j_common_ptr p_jpeg)
> +{
> +    encoder_sys_t *p_sys = (encoder_sys_t *)p_jpeg->err;

Here you could use decoder_sys_t with a comment that only the common 
members can be used (which includes p_sys->p_jpeg in my patch).

> +    p_sys->err.output_message(p_jpeg);
> +    longjmp(p_sys->setjmp_buffer, 1);
> +}
> +
> +/*
> + * Emit message error handler for libjpeg
> + */
> +static void encoder_error_message(j_common_ptr p_jpeg)
> +{
> +    char error_msg[JMSG_LENGTH_MAX];
> +
> +    encoder_sys_t *p_sys = (encoder_sys_t *)p_jpeg->err;
> +    p_sys->err.format_message(p_jpeg, error_msg);
> +    msg_Err(p_sys->p_obj, "%s", error_msg);

Same comment here, p_obj is common to both.

> +}
> +
> +static block_t *EncodeBlock(encoder_t *p_enc, picture_t *p_pic)
> +{
> +    encoder_sys_t *p_sys = p_enc->p_sys;
> +
> +    const int bytesPerPixel = p_enc->fmt_out.video.i_bits_per_pixel ?
> +                                p_enc->fmt_out.video.i_bits_per_pixel / 8 : 3;

You should rather set *input* video format in Open

> +    const int blocksize = bytesPerPixel * p_enc->fmt_in.video.i_visible_width * p_enc->fmt_in.video.i_visible_height;
> +
> +    block_t *p_block = block_Alloc(blocksize);
> +    if (p_block == NULL)
> +    {
> +        return NULL;
> +    }
> +
> +    struct jpeg_compress_struct p_jpeg;
> +    JSAMPIMAGE p_row_pointers = NULL;
> +
> +    p_jpeg.err = jpeg_std_error(&p_sys->err);
> +    p_sys->err.error_exit = encoder_error_exit;
> +    p_sys->err.output_message = encoder_error_message;
> +
> +    /* libjpeg longjmp's there in case of error */
> +    if (setjmp(p_sys->setjmp_buffer))
> +    {
> +        goto error;
> +    }
> +
> +    jpeg_create_compress(&p_jpeg);
> +    jpeg_mem_dest(&p_jpeg, &p_block->p_buffer, &p_block->i_buffer);
> +
> +    p_jpeg.image_width = p_enc->fmt_in.video.i_visible_width;
> +    p_jpeg.image_height = p_enc->fmt_in.video.i_visible_height;
> +    p_jpeg.input_components = bytesPerPixel;
> +    p_jpeg.in_color_space = JCS_YCbCr;
> +
> +    jpeg_set_defaults(&p_jpeg);
> +    jpeg_set_colorspace(&p_jpeg, JCS_YCbCr);
> +
> +    p_jpeg.raw_data_in = TRUE;
> +    p_jpeg.do_fancy_downsampling = FALSE;
> +
> +    jpeg_set_quality(&p_jpeg, p_sys->i_quality, TRUE);
> +
> +    jpeg_start_compress(&p_jpeg, TRUE);
> +

This should be done in Open, then you don't even need to store i_quality and 
both modules can use the same context.

> +    /* Encode picture */
> +    p_row_pointers = malloc(sizeof(JSAMPIMAGE) * p_pic->i_planes);
> +    for (int i = 0; i < p_pic->i_planes; i++)
> +    {
> +        p_row_pointers[i] = malloc(sizeof(JSAMPARRAY) * p_pic->p[i].i_lines);

memleak

> +        for (int j = 0; j < p_pic->p[i].i_lines; j++)
> +        {
> +            p_row_pointers[i][j] = p_pic->p[i].p_pixels + p_pic->p[i].i_pitch * j;
> +        }
> +    }
> +
> +    while (p_jpeg.next_scanline < p_jpeg.image_height)
> +    {
> +        jpeg_write_raw_data(&p_jpeg, p_row_pointers, p_jpeg.max_v_samp_factor * DCTSIZE);
> +        for (int i = 0; i < p_pic->i_planes; i++)
> +        {
> +            p_row_pointers[i] += p_jpeg.comp_info[i].v_samp_factor * DCTSIZE;
> +        }
> +    }

Can you rather operate directly on p_pixels ?

> +
> +    jpeg_finish_compress(&p_jpeg);
> +    jpeg_destroy_compress(&p_jpeg);
> +
> +    free(p_row_pointers);
> +
> +    return p_block;
> +
> +error:
> +    jpeg_destroy_compress(&p_jpeg);
> +
> +    free(p_row_pointers);
> +
> +    block_Release(p_block);
> +
> +    return NULL;
> +}
> +
> +/*
> + * jpeg encoder destruction
> + */
> +static void CloseEncoder(vlc_object_t *p_this)
> +{
> +    encoder_t *p_enc = (encoder_t *)p_this;
> +    encoder_sys_t *p_sys = p_enc->p_sys;
> +
> +    free(p_sys);
> +}



More information about the vlc-devel mailing list