[vlc-devel] [PATCH] png: add encoder
Rafaël Carré
funman at videolan.org
Wed Feb 19 08:39:14 CET 2014
Hello, a few comments below
On 02/19/14 08:27, Tristan Matthews wrote:
> This patch adds a basic encoder to the png module (using libpng).
>
> ---
> modules/codec/png.c | 181 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 178 insertions(+), 3 deletions(-)
>
> diff --git a/modules/codec/png.c b/modules/codec/png.c
> index b4d6ef4..7bb0320 100644
> --- a/modules/codec/png.c
> +++ b/modules/codec/png.c
> @@ -49,6 +49,20 @@ static void CloseDecoder ( vlc_object_t * );
>
> static picture_t *DecodeBlock ( decoder_t *, block_t ** );
>
> +/*
> + * png encoder descriptor
> + */
> +struct encoder_sys_t
> +{
> + int i_blocksize;
> + bool b_error;
> +};
> +
> +static int OpenEncoder(vlc_object_t *);
> +static void CloseEncoder(vlc_object_t *);
> +
> +static block_t *EncodeBlock(encoder_t *, picture_t *);
> +
> /*****************************************************************************
> * Module descriptor
> *****************************************************************************/
> @@ -59,6 +73,14 @@ vlc_module_begin ()
> set_capability( "decoder", 1000 )
> set_callbacks( OpenDecoder, CloseDecoder )
> add_shortcut( "png" )
> +
> + /* encoder submodule */
> + add_submodule()
> + add_shortcut("png")
> + set_section(N_("Encoding"), NULL)
> + set_description(N_("PNG video encoder"))
> + set_capability("encoder", 1000)
> + set_callbacks(OpenEncoder, CloseEncoder)
> vlc_module_end ()
>
> /*****************************************************************************
> @@ -101,19 +123,51 @@ static void user_read( png_structp p_png, png_bytep data, png_size_t i_length )
> p_block->i_buffer -= i_length;
> }
>
> +static void user_flush( png_structp p_png )
> +{
> + /* noop */
> + (void) p_png;
> +}
> +
> +static void user_write( png_structp p_png, png_bytep data, png_size_t i_length )
> +{
> + block_t *p_block = (block_t *)png_get_io_ptr( p_png );
> + if( i_length > p_block->i_buffer ) {
can you use block_Realloc and store the result ?
> + png_error( p_png, "not enough data" );
> + return;
> + }
> +
> + memcpy( p_block->p_buffer, data, i_length );
> + p_block->p_buffer += i_length;
> + p_block->i_buffer -= i_length;
> +}
> +
> -static void user_error( png_structp p_png, png_const_charp error_msg )
> +static void user_decoder_error( png_structp p_png, png_const_charp error_msg )
You can use the same function for both decoder and encoder, see my comments
on encoder_error_message in
https://mailman.videolan.org/pipermail/vlc-devel/2014-January/096466.html
> {
> decoder_t *p_dec = (decoder_t *)png_get_error_ptr( p_png );
> p_dec->p_sys->b_error = true;
> msg_Err( p_dec, "%s", error_msg );
> }
>
> -static void user_warning( png_structp p_png, png_const_charp warning_msg )
> +static void user_encoder_error( png_structp p_png, png_const_charp error_msg )
> +{
> + encoder_t *p_enc = (encoder_t*)png_get_error_ptr( p_png );
> + p_enc->p_sys->b_error = true;
> + msg_Err( p_enc, "%s", error_msg );
> +}
> +
> +static void user_decoder_warning( png_structp p_png, png_const_charp warning_msg )
> {
> decoder_t *p_dec = (decoder_t *)png_get_error_ptr( p_png );
> msg_Warn( p_dec, "%s", warning_msg );
> }
>
> +static void user_encoder_warning( png_structp p_png, png_const_charp warning_msg )
> +{
> + encoder_t *p_enc = (encoder_t *)png_get_error_ptr( p_png );
> + msg_Warn( p_enc, "%s", warning_msg );
> +}
Same for these
> /****************************************************************************
> * DecodeBlock: the whole thing
> ****************************************************************************
> @@ -172,7 +226,7 @@ static picture_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
> goto error;
>
> png_set_read_fn( p_png, (void *)p_block, user_read );
> - png_set_error_fn( p_png, (void *)p_dec, user_error, user_warning );
> + png_set_error_fn( p_png, (void *)p_dec, user_decoder_error, user_decoder_warning );
>
> png_read_info( p_png, p_info );
> if( p_sys->b_error ) goto error;
> @@ -253,3 +307,124 @@ static void CloseDecoder( vlc_object_t *p_this )
>
> free( p_sys );
> }
> +
> +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_PNG )
> + return VLC_EGENERIC;
> +
> + /* Allocate the memory needed to store the encoder's structure */
> + if( ( p_enc->p_sys = malloc(sizeof(encoder_sys_t)) ) == NULL )
> + return VLC_ENOMEM;
Honestly I try to remove assignements and comparisons merged together
when I see them.
> + p_enc->p_sys->i_blocksize = 3 * p_enc->fmt_in.video.i_visible_width *
> + p_enc->fmt_in.video.i_visible_height;
> +
> + p_enc->fmt_in.i_codec = VLC_CODEC_RGB24;
> + p_enc->fmt_in.video.i_rmask = 0x000000ff;
> + p_enc->fmt_in.video.i_gmask = 0x0000ff00;
> + p_enc->fmt_in.video.i_bmask = 0x00ff0000;
> + p_enc->pf_encode_video = EncodeBlock;
> +
> + return VLC_SUCCESS;
> +}
> +
> +/*
> + * EncodeBlock
> + */
> +static block_t *EncodeBlock(encoder_t *p_enc, picture_t *p_pic)
> +{
> + encoder_sys_t *p_sys = p_enc->p_sys;
> +
> + if( unlikely( !p_pic ) )
> + {
> + return NULL;
> + }
> +
> + block_t *p_block = block_Alloc( p_sys->i_blocksize );
> + if( p_block == NULL )
> + {
> + return NULL;
> + }
> +
> + png_structp p_png = png_create_write_struct( PNG_LIBPNG_VER_STRING, 0, 0, 0 );
> + if( p_png == NULL )
> + {
> + block_Release( p_block );
> + return NULL;
> + }
> +
> + /* save buffer start */
> + void *start = p_block->p_buffer;
> +
> + p_sys->b_error = false;
> +
> + /* libpng longjmp's there in case of error */
> + if( setjmp( png_jmpbuf( p_png ) ) )
> + goto error;
> +
> + png_set_write_fn( p_png, p_block, user_write, user_flush );
> + png_set_error_fn( p_png, p_enc, user_encoder_error, user_encoder_warning );
> +
> + png_infop p_info = png_create_info_struct( p_png );
> + if( p_info == NULL )
> + {
goto error ?
maybe you need a if (p_info) in error: case.
> + png_destroy_write_struct( &p_png, NULL );
> + block_Release( p_block );
> + return NULL;
> + }
> +
> + png_infop p_end_info = png_create_info_struct( p_png );
> + if( p_end_info == NULL ) goto error;
> +
> + const unsigned i_width = p_enc->fmt_in.video.i_visible_width;
> + const unsigned i_height = p_enc->fmt_in.video.i_visible_height;
> +
> + png_set_IHDR( p_png, p_info,
> + i_width,
> + i_height,
> + 8, PNG_COLOR_TYPE_RGB, PNG_INTERLACE_NONE,
> + PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT );
> + if( p_sys->b_error ) goto error;
> +
> + png_write_info( p_png, p_info );
> + if( p_sys->b_error ) goto error;
> +
> + /* Encode picture */
> +
> + for( unsigned i = 0; i < i_height; i++ )
> + {
> + png_write_row( p_png, p_pic->p->p_pixels + (i_width * i * 3));
> + if( p_sys->b_error ) goto error;
> + }
> +
> + png_write_end( p_png, p_end_info );
> + if( p_sys->b_error ) goto error;
> +
> + png_destroy_write_struct( &p_png, &p_info );
> +
> + /* restore original buffer position */
> + p_block->p_buffer = start;
Why would it be modified?
> + return p_block;
> +
> + error:
> +
> + png_destroy_write_struct( &p_png, &p_info );
> +
> + p_block->p_buffer = start;
Not needed if the block is freed.
> + block_Release(p_block);
> + return NULL;
> +}
> +
> +/*****************************************************************************
> + * CloseEncoder: png 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