[FFmpeg-devel] [PATCH] avcodec/h264_ps: use the AVBufferPool API to allocate parameter set buffers

[James Almer]

On 1/20/2018 11:33 PM, Michael Niedermayer wrote:
> On Sat, Jan 20, 2018 at 06:49:29PM -0300, James Almer wrote:
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> Similar rationale as hevc. With up to 32 sps and 256 pps, this may
>> come in handy when parsing raw streams.
>>
>>  libavcodec/h264_parser.c   |  3 ++-
>>  libavcodec/h264_ps.c       | 22 ++++++++++++++++++++--
>>  libavcodec/h264_ps.h       |  5 +++++
>>  libavcodec/h264dec.c       |  3 +++
>>  libavcodec/mediacodecdec.c |  4 ++++
>>  5 files changed, 34 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
>> index 65d9d44b50..fa6777fc05 100644
>> --- a/libavcodec/h264_parser.c
>> +++ b/libavcodec/h264_parser.c
>> @@ -697,7 +697,8 @@ static av_cold int init(AVCodecParserContext *s)
>>      p->reference_dts = AV_NOPTS_VALUE;
>>      p->last_frame_num = INT_MAX;
>>      ff_h264dsp_init(&p->h264dsp, 8, 1);
>> -    return 0;
>> +
>> +    return ff_h264_ps_init(&p->ps);
>>  }
>>  
>>  AVCodecParser ff_h264_parser = {
>> diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
>> index 8d1ef831fa..50dbabdb8b 100644
>> --- a/libavcodec/h264_ps.c
>> +++ b/libavcodec/h264_ps.c
>> @@ -314,6 +314,21 @@ static int decode_scaling_matrices(GetBitContext *gb, const SPS *sps,
>>      return ret;
>>  }
>>  
>> +int ff_h264_ps_init(H264ParamSets *ps)
>> +{
>> +    ps->sps_pool = av_buffer_pool_init(sizeof(*ps->sps), av_buffer_allocz);
>> +    ps->pps_pool = av_buffer_pool_init(sizeof(*ps->pps), av_buffer_allocz);
>> +
>> +    if (!ps->sps_pool || !ps->pps_pool) {
>> +        av_buffer_pool_uninit(&ps->sps_pool);
>> +        av_buffer_pool_uninit(&ps->pps_pool);
>> +
>> +        return AVERROR(ENOMEM);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>  void ff_h264_ps_uninit(H264ParamSets *ps)
>>  {
>>      int i;
>> @@ -327,6 +342,9 @@ void ff_h264_ps_uninit(H264ParamSets *ps)
>>      av_buffer_unref(&ps->sps_ref);
>>      av_buffer_unref(&ps->pps_ref);
>>  
>> +    av_buffer_pool_uninit(&ps->sps_pool);
>> +    av_buffer_pool_uninit(&ps->pps_pool);
>> +
>>      ps->pps = NULL;
>>      ps->sps = NULL;
>>  }
>> @@ -341,7 +359,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
>>      SPS *sps;
>>      int ret;
>>  
>> -    sps_buf = av_buffer_allocz(sizeof(*sps));
>> +    sps_buf = av_buffer_pool_get(ps->sps_pool);
>>      if (!sps_buf)
>>          return AVERROR(ENOMEM);
>>      sps = (SPS*)sps_buf->data;
>> @@ -738,7 +756,7 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
>>          return AVERROR_INVALIDDATA;
>>      }
>>  
>> -    pps_buf = av_buffer_allocz(sizeof(*pps));
>> +    pps_buf = av_buffer_pool_get(ps->pps_pool);
> 
> this seems to remove the memset(0) unless iam missing something

Isn't it enough using av_buffer_allocz() as the alloc function when
initializing the pool?
If the buffers are not cleared when returned to the pool to be reused
then I could change the alloc function to av_buffer_alloc() and call
memset(0) here.

> is this intended?

Not really. My intention is to still get a zeroed buffer as it always did.

> has someone checked this to be safe ?
> 
> 
> [...]
>> --- a/libavcodec/h264dec.c
>> +++ b/libavcodec/h264dec.c
>> @@ -326,6 +326,9 @@ static int h264_init_context(AVCodecContext *avctx, H264Context *h)
>>  
>>      ff_h264_sei_uninit(&h->sei);
>>  
>> +    if (ff_h264_ps_init(&h->ps) < 0)
>> +        return AVERROR(ENOMEM);
> 
> This probably should forward the returned error code

Changed locally.

> 
> also a memcmp on the data might be worth looking into to avoid re-parsing of
> unchanged PS
> (theres already a memcmp in libavcodec/h264_ps.c)
> 
> [...]
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>