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

[Tristan Matthews]

On Fri, Jul 4, 2014 at 10:00 AM, Rémi Denis-Courmont <remi at remlab.net>
wrote:

> 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?


av_pkt.data and av_pkt.size must now be initialized to 0 before being
passed to avcodec_encode_video2. It could be done explicitly (i.e.
av_pkt.size = 0 av_pkt.data = NULL), I just wrote it this way to be
consistent with the style used in EncodeAudio.


>
>
>       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.


Yeah I had the same thought, but I wasn't sure if it was possible. Aside
from overloading the block's pf_release function pointer to call
av_free_packet, how could the AVPacket be wrapped without breaking callers?
Do we have any examples of this kind of thing already in VLC?

Best,
Tristan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20140704/6fce4ca3/attachment.html>