[vlc-devel] [PATCH 2/7] picture_pool: add the video format requested when waiting for a picture
Steve Lhomme
robux4 at ycbcr.xyz
Mon Nov 9 09:04:49 CET 2020
On 2020-11-06 18:00, Rémi Denis-Courmont wrote:
> Le perjantaina 6. marraskuuta 2020, 16.42.09 EET Steve Lhomme a écrit :
>> In push mode it's convenient to keep the same pool of buffers when only
>> small changes to the video format occur.
>
> This makes no sense to me. In push mode, the allocator is also the format
> controller. It is free to switch pool if it needs to, or if it is simpler, or
> to keep the same pool.
Yes, except 99% of the code doesn't do that (nvdec being a rare
exception). The pool is invisible to the decoder, it is handled in
decoder_helpers.c.
The good news is that it makes easier to change this for 99% of the usage.
> The need to reallocate buffers is anyway in general dependent on the specific
> allocator/decoder, so I don't even follow how passing the format in generic
> functions helps at all.
>
> I also cannot relate this to any plan that was agreed upon in the past.
The generic/clean way is to create a new pool. But it is creating a lot
of stress on the memory. Imagine decoding 4K HEVC on 32 cores. You have
a huge pool. And you would allocate such a pool every time the video
format changes. That may happen often with HDR metadata (in HDR10+ I
suppose that would be the case). Same thing with interlacing in push.
Then at some point you could have 4/10/whatever the same huge pool
allocated in memory.
The picture pools exist for 2 reasons:
- preallocate the buffers so we don't run out of memory midstream
- pace the decoder so it doesn't allocate an infinite amount of memory
None of this is related to the video_format_t. It has everything to do
with the allocated buffers (decoder width/height and planes for the
chroma). The rest of the video_format_t is just attached to the buffers
when we use them. The fact what is in the video_format_t changes is
useless to the pool.
IMO the picture pool should only deal with the buffer pooling and not
the video_format_t attached to it. This patch is one step in that
direction. I can do the rest if needed.
That's also what the nvdec_pool_t and the va_pool_t do. They deal with
surface pooling and don't care about the video_format_t. I think we
should unify all this.
More information about the vlc-devel
mailing list