Re: [FFmpeg-devel] [RFC][PATCH 1/4] avutil/frame: add an internal field to store the size of AVFrame

[James Almer <jamrial@gmail.com>]

On 2/12/2022 9:32 AM, Paul B Mahol wrote:
> On Sat, Feb 12, 2022 at 1:28 PM James Almer <jamrial@gmail.com> wrote:
> 
>> On 2/12/2022 9:25 AM, Paul B Mahol wrote:
>>> On Sat, Feb 12, 2022 at 1:14 AM James Almer <jamrial@gmail.com> wrote:
>>>
>>>> This is unfortunately needed to remove (or reduce the awfulness) of
>> certain
>>>> modules violating the AVFrame API and using sizeof(AVFrame).
>>>>
>>>
>>> Which modules?
>>
>> Anything using wrapped_avframe, so wrapped_avframe decoder and encoder,
>> and vapoursynth demuxer, which exports wrapped_avframe packets.
>>
>> I gave porting the latter into a video source filter some time ago a
>> try, which would let us remove the "demuxer", but never sent a patch. I
>> can dig it and do that, but it will not change the fact both the
>> wrapped_avframe decoder and encoder are still using sizeof(AVFrame).
>>
> 
> There was some talk that out VS demuxer is slower than it should be.

Really? It's literally a zero copy implementation. It was the entire 
reason why it was written using wrapped_avframe instead of the proper 
rawvideo. I don't see it getting any faster than it is from our side.

> 
> 
>>
>>>
>>>
>>>> With this, the sizeof(AVFrame) value of the libavutil loaded at runtime
>>>> can be
>>>> used instead of the compile time value of whatever library included
>> frame.h
>>>>
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>> This is sucks, but at least less so than the current situation.
>>>>
>>>> I don't see wrapped_avframe going away anytime soon, so something must
>> be
>>>> done,
>>>> and last time i tried to change how the packets are generated my
>> approach
>>>> was
>>>> shut down, so here's another attempt.
>>>>
>>>>    libavutil/frame.c          |  3 +++
>>>>    libavutil/frame_internal.h | 33 +++++++++++++++++++++++++++++++++
>>>>    2 files changed, 36 insertions(+)
>>>>    create mode 100644 libavutil/frame_internal.h
>>>>
>>>> diff --git a/libavutil/frame.c b/libavutil/frame.c
>>>> index 8997c85e35..a63d2979db 100644
>>>> --- a/libavutil/frame.c
>>>> +++ b/libavutil/frame.c
>>>> @@ -23,6 +23,7 @@
>>>>    #include "cpu.h"
>>>>    #include "dict.h"
>>>>    #include "frame.h"
>>>> +#include "frame_internal.h"
>>>>    #include "imgutils.h"
>>>>    #include "mem.h"
>>>>    #include "samplefmt.h"
>>>> @@ -33,6 +34,8 @@
>>>>                   (frame)->channels == \
>>>>
>> av_get_channel_layout_nb_channels((frame)->channel_layout))
>>>>
>>>> +const size_t avpriv_avframe_size = sizeof(AVFrame);
>>>> +
>>>>    #if FF_API_COLORSPACE_NAME
>>>>    const char *av_get_colorspace_name(enum AVColorSpace val)
>>>>    {
>>>> diff --git a/libavutil/frame_internal.h b/libavutil/frame_internal.h
>>>> new file mode 100644
>>>> index 0000000000..07c246f86a
>>>> --- /dev/null
>>>> +++ b/libavutil/frame_internal.h
>>>> @@ -0,0 +1,33 @@
>>>> +/*
>>>> + * This file is part of FFmpeg.
>>>> + *
>>>> + * FFmpeg is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU Lesser General Public
>>>> + * License as published by the Free Software Foundation; either
>>>> + * version 2.1 of the License, or (at your option) any later version.
>>>> + *
>>>> + * FFmpeg is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>> + * Lesser General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU Lesser General Public
>>>> + * License along with FFmpeg; if not, write to the Free Software
>>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>>> 02110-1301 USA
>>>> + */
>>>> +
>>>> +#ifndef AVUTIL_FRAME_INTERNAL_H
>>>> +#define AVUTIL_FRAME_INTERNAL_H
>>>> +
>>>> +#include <stdint.h>
>>>> +
>>>> +#include "frame.h"
>>>> +
>>>> +/**
>>>> + * sizeof(AVFrame). If you think you need to use it, then you need to
>>>> change
>>>> + * your code so you don't instead.
>>>> + * Meant for exceptions like wrapped_avframe.
>>>> + */
>>>> +extern const size_t avpriv_avframe_size;
>>>> +
>>>> +#endif /* AVUTIL_FRAME_INTERNAL_H */
>>>> --
>>>> 2.35.1
>>>>
>>>> _______________________________________________
>>>> ffmpeg-devel mailing list
>>>> ffmpeg-devel@ffmpeg.org
>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>
>>>> To unsubscribe, visit link above, or email
>>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".