[FFmpeg-devel] [PATCH] lavc/vaapi_decode: Set surfaces reference pool size according to SPS for H.264/HEVC

[Fu, Linjie]

Hi,
> On Nov 18, 2019, at 07:40, Mark Thompson <sw at jkqxz.net> wrote:
> 
> On 06/11/2019 04:53, Linjie Fu wrote:
>> Set surfaces pool used for storing reference frames dynamically
>> according to SPS.(reference framecount, reordered frame number, etc)
>> 
>> Compared to a fixed pool size for H.264 and HEVC, the usage of
>> GPU memory could be improved.
>> 
>> CMD:
>> ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128
>>        -i bbb_sunflower_1080p_30fps_normal.mp4 -f null -
>> Source:
>>    https://download.blender.org/demo/movies/BBB/
>> 
>> GEM Object memory usage:
>>    watch cat /sys/kernel/debug/dri/0/i915_gem_objects
>> Before:
>>    ~112M
>> After:
>>    ~80M
>> 
>> Signed-off-by: Linjie Fu <linjie.fu at intel.com>
>> ---
>> libavcodec/vaapi_decode.c | 39 ++++++++++++++++++---------------------
>> libavcodec/vaapi_decode.h |  3 +++
>> libavcodec/vaapi_h264.c   | 11 ++++++++++-
>> libavcodec/vaapi_hevc.c   | 11 ++++++++++-
>> libavcodec/vaapi_vp8.c    |  8 +++++++-
>> libavcodec/vaapi_vp9.c    |  8 +++++++-
>> 6 files changed, 55 insertions(+), 25 deletions(-)
> 
> The number of required reference frames is a property of the codec, not a property of the hardware used to decode it.
>  I think make a function to determine the required size given an AVCodecContext and then call it in vaapi_decode_make_config() rather than adding all of these extra functions inside the VAAPI code.  (And then you can call it in ff_dxva2_common_frame_params() as well.)

A property of the codec seems more reasonable, and the modifications would be more general. Thanks for this input.

> Relatedly, do we believe that all H.264 streams which people might want to decode are actually marked correctly here?  The failure mode will be bad if we run out.

I think it’s a trade off. On most cases the streams are reliable, such modifications would save GEM object usage (a lot if multiple processes are running)

Maybe set to the default value for some obvious mistakes would be a good way to handle the unexpected streams? Like a minimum reference if the streams declare 0 reference needed?

I’ll update like this if it’s the right thing to do.

Thanks,

- linjie