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

[Paul B Mahol]

On Sat, Feb 12, 2022 at 1:33 PM James Almer <jamrial at gmail.com> wrote:

> On 2/12/2022 9:32 AM, Paul B Mahol wrote:
> > On Sat, Feb 12, 2022 at 1:28 PM James Almer <jamrial at 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 at 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.
>
> Slowness is probably not in that part but how VS API is used.


> >
> >
> >>
> >>>
> >>>
> >>>> 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 at 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 at ffmpeg.org
> >>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>>
> >>>> To unsubscribe, visit link above, or email
> >>>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >>>>
> >>> _______________________________________________
> >>> ffmpeg-devel mailing list
> >>> ffmpeg-devel at ffmpeg.org
> >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>
> >>> To unsubscribe, visit link above, or email
> >>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >>
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel at ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >>
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>