[FFmpeg-devel] [PATCH] lavu/buffer: add release function

Reimar Döffinger Reimar.Doeffinger at gmx.de
Sun Mar 2 18:39:37 CET 2014

On 02.03.2014, at 18:14, Michael Niedermayer <michaelni at gmx.at> wrote:

> On Sun, Mar 02, 2014 at 05:48:11PM +0100, Lukasz Marek wrote:
>> On 02.03.2014 17:14, Reimar Döffinger wrote:
>>> On 28.02.2014, at 01:21, Lukasz Marek <lukasz.m.luki at gmail.com> wrote:
>>>>> Keep in mind that a buffer can come from a user application. There is
>>>>> no way you could free such a buffer correctly by destroying the buffer
>>>>> reference and just returning the raw data.
>>> This buffer might be allocated on some special hardware, and to allocate/lock or deallocate/unlock it you need to write some magic registers.
>>> What happens if you use this function on such a buffer?
>>> I can see only two things happen:
>>> 1) you call the custom free function, then all following code trying to use it will cause bus errors or worse
>> I'm not sure I understand but if user operates on data they released
>> with free function its just their fault. free() is dangerous in the
>> same way.
>> I assumed user will not release buffer if don't know how to delete it.

If we have public API that has such assumptions it needs big red flashing warning signs, because not only can it surprise users it can mislead FFmpeg developers to just reuse these "assumptions". Personally I think it is a horrible way to design an API.

> yes, also it would be possible to check if the free function points
> to the standard free() based one in release buffer.
> (this check is not 100% fool proof but it will only error onto the
> safe  side)
> The code already contains support for using memdup which could be
> used if its some fancy hw free

I agree this would be possible, though it might mean it is a bit unreliable performance-wise, and the code would be a bit messier.
Definitely something worth considering, though maybe a bit overkill for a function nobody knows if it will ever be used...

More information about the ffmpeg-devel mailing list