[FFmpeg-devel] [PATCH] avcodec/hevc: reduce memory used by the SAO

[Christophe Gisquet]

Hi,

2015-02-05 19:19 GMT+01:00 Michael Niedermayer <michaelni at gmx.at>:
>> +    frame->width  = FFALIGN(4 * sps->width, FF_INPUT_BUFFER_PADDING_SIZE);
>
> FF_INPUT_BUFFER_PADDING_SIZE is not guranteed to be a power of 2 but
> FFALIGN needs a power of 2

I don't think the padding is actually needed because it's just plain C
accessing this. There's a dsp restore function but that is unlikely to
get simd.

So, maybe that is sufficient: width  = 4 * sps->width +
FF_INPUT_BUFFER_PADDING_SIZE.
Same for the other allocation.

>>  static int set_sps(HEVCContext *s, const HEVCSPS *sps)
>>  {
>>      #define HWACCEL_MAX (CONFIG_HEVC_DXVA2_HWACCEL)
>> @@ -335,19 +352,14 @@ static int set_sps(HEVCContext *s, const HEVCSPS *sps)
>>      ff_videodsp_init (&s->vdsp,    sps->bit_depth);
>>
>>      if (sps->sao_enabled && !s->avctx->hwaccel) {
>> -        int c_count = (sps->chroma_format_idc != 0) ? 3 : 1;
>> -        int c_idx;
>> -
>> -        for(c_idx = 0; c_idx < c_count; c_idx++) {
>> -            int w = sps->width >> sps->hshift[c_idx];
>> -            int h = sps->height >> sps->vshift[c_idx];
>> -            s->sao_pixel_buffer_h[c_idx] =
>> -                av_malloc((w * 2 * sps->ctb_height) <<
>> -                          sps->pixel_shift);
>> -            s->sao_pixel_buffer_v[c_idx] =
>> -                av_malloc((h * 2 * sps->ctb_width) <<
>> -                          sps->pixel_shift);
>> -        }
>> +        av_frame_unref(s->tmp_frame[0]);
>> +        av_frame_unref(s->tmp_frame[1]);
>> +        s->sao_frame[0] = NULL;
>> +        s->sao_frame[1] = NULL;
>> +        if ( (ret = get_buffer_sao(s, sps)) < 0 )
>> +            goto fail;
>> +        s->sao_frame[0] = s->tmp_frame[0];
>> +        s->sao_frame[1] = s->tmp_frame[1];
>
> I dont understand this code
> tmp_frame is allocated then the pointer copied into sao_frame
> later then tmp_frame is unreferenced which makes sao_frame a stale
> pointer if its still pointing to the same memory
> it seems the code works though with make fate even under valgrind,
> so maybe iam missing something but wouldnt a simply av_freep() with
> the existing code achive the same ?

At first I thought this was a threading bug, eg a thread accessing
something no longer valid.
That would have been the reason why there was a reference-counted
frame. But the sequence having an issue is one where there are
multiple parameter sets, so this code gets executed twice. Because the
code didn't free the buffers, there was a leak

So, you're right, it's just a fancy way of freeing the buffers. As you
said, one can probably add the needed av_freep, something like the
attached patch.

It is untested, though, as I can't test it right now.

-- 
Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-hevc-free-sao-buffers-when-receiving-a-new-SPS.patch
Type: text/x-patch
Size: 1398 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150205/4f6af3b4/attachment.bin>