[vlc-devel] [PATCH] avcodec: don't guess size when encoding video

[Rémi Denis-Courmont]

Le 2014-07-04 16:54, Tristan Matthews a écrit :
> Fixes #11605
> ---
>  modules/codec/avcodec/encoder.c | 31 +++++++++++--------------------
>  1 file changed, 11 insertions(+), 20 deletions(-)
>
> diff --git a/modules/codec/avcodec/encoder.c
> b/modules/codec/avcodec/encoder.c
> index a6fcf66..3646b6e 100644
> --- a/modules/codec/avcodec/encoder.c
> +++ b/modules/codec/avcodec/encoder.c
> @@ -1013,19 +1013,6 @@ static block_t *EncodeVideo( encoder_t *p_enc,
> picture_t *p_pict )
>  {
>      encoder_sys_t *p_sys = p_enc->p_sys;
>      int i_plane;
> -    /* Initialize the video output buffer the first time.
> -     * This is done here instead of OpenEncoder() because we need 
> the actual
> -     * bits_per_pixel value, without having to assume anything.
> -     */
> -    const int bitsPerPixel = p_enc->fmt_out.video.i_bits_per_pixel ?
> -                         p_enc->fmt_out.video.i_bits_per_pixel :
> -                         p_sys->p_context->bits_per_coded_sample ?
> -                         p_sys->p_context->bits_per_coded_sample :
> -                         24;
> -    const int blocksize = __MAX( FF_MIN_BUFFER_SIZE, ( bitsPerPixel
> * p_sys->p_context->height * p_sys->p_context->width ) / 8 + 200 );
> -    block_t *p_block = block_Alloc( blocksize );
> -    if( unlikely(p_block == NULL) )
> -        return NULL;
>
>      AVFrame *frame = NULL;
>      if( likely(p_pict) ) {
> @@ -1090,7 +1077,6 @@ static block_t *EncodeVideo( encoder_t *p_enc,
> picture_t *p_pict )
>              {
>                  msg_Warn( p_enc, "almost fed libavcodec with two
> frames with "
>                            "the same PTS (%"PRId64 ")", frame->pts );
> -                block_Release( p_block );
>                  return NULL;
>              }
>              else if ( p_sys->i_last_pts > frame->pts )
> @@ -1098,7 +1084,6 @@ static block_t *EncodeVideo( encoder_t *p_enc,
> picture_t *p_pict )
>                  msg_Warn( p_enc, "almost fed libavcodec with a frame
> in the "
>                           "past (current: %"PRId64 ", last: 
> %"PRId64")",
>                           frame->pts, p_sys->i_last_pts );
> -                block_Release( p_block );
>                  return NULL;
>              }
>              else
> @@ -1108,21 +1093,27 @@ static block_t *EncodeVideo( encoder_t
> *p_enc, picture_t *p_pict )
>          frame->quality = p_sys->i_quality;
>      }
>
> -    AVPacket av_pkt;
> +    AVPacket av_pkt = {0};

Why is this needed now?

>      int is_data;
>
>      av_init_packet( &av_pkt );
> -    av_pkt.data = p_block->p_buffer;
> -    av_pkt.size = p_block->i_buffer;
>
>      if( avcodec_encode_video2( p_sys->p_context, &av_pkt, frame,
> &is_data ) < 0
>       || is_data == 0 )
>      {
> -        block_Release( p_block );
>          return NULL;
>      }
>
> -    p_block->i_buffer = av_pkt.size;
> +    block_t *p_block = block_Alloc( av_pkt.size );
> +    if( unlikely(p_block == NULL) )
> +    {
> +        av_free_packet( &av_pkt );
> +        return NULL;
> +    }
> +
> +    memcpy( p_block->p_buffer, av_pkt.data, av_pkt.size );
> +    av_free_packet( &av_pkt );

"memcpy() is evil." IMHO, the AVPacket should be wrapped into a block_t 
rather than copied.

> +
>      p_block->i_length = av_pkt.duration / 
> p_sys->p_context->time_base.den;
>      p_block->i_pts = av_pkt.pts;
>      p_block->i_dts = av_pkt.dts;

-- 
Rémi Denis-Courmont