Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: James Almer <jamrial@gmail.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [RFC][PATCH 1/4] avutil/frame: add an internal field to store the size of AVFrame
Date: Sat, 12 Feb 2022 08:58:22 -0300
Message-ID: <4bb99492-a8cb-7faf-e7dd-94bd7d7461b7@gmail.com> (raw)
In-Reply-To: <AM7PR03MB6660A29EC523C7160B64A4B78F319@AM7PR03MB6660.eurprd03.prod.outlook.com>

On 2/12/2022 3:45 AM, Andreas Rheinhardt wrote:
> James Almer:
>> This is unfortunately needed to remove (or reduce the awfulness) of certain
>> modules violating the AVFrame API and using sizeof(AVFrame).
>> 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.
>>
> 
> Where can I find this earlier approach?

I think it was 
https://ffmpeg.org/pipermail/ffmpeg-devel/2021-March/277762.html and 
https://ffmpeg.org/pipermail/ffmpeg-devel/2021-March/277759.html (Which 
you looked at, back then).

> (Also why don't we just switch to something like what is done for
> uncoded frames in libavformat/mux.c?)

What would that be? Remember that AVPackets with wrapped AVFrames are 
propagated to the user if the "codec" is wrapped_avframe. We have not 
forbidden them to look at any specific field, including size, so any 
change will probably be backwards incompatible.
With uncoded frame the user passes an AVFrame to lavf and never sees or 
deals with it being added to a packet.

> 
>>   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"
> 
> This header is completely unnecessary.
> 
>> +
>> +/**
>> + * 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 */
> 
> _______________________________________________
> 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".

  reply	other threads:[~2022-02-12 11:58 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-12  0:12 James Almer
2022-02-12  0:12 ` [FFmpeg-devel] [PATCH 2/4] avcodec/wrapped_avframe: don't allocate an AVFrame twice James Almer
2022-02-12  0:13 ` [FFmpeg-devel] [PATCH 3/4] avcodec/wrapped_avframe: stop hardcoding sizeof(AVFrame) James Almer
2022-02-12  0:13 ` [FFmpeg-devel] [PATCH 4/4] avformat/vapoursynth: " James Almer
2022-02-12  5:29 ` [FFmpeg-devel] [RFC][PATCH 1/4] avutil/frame: add an internal field to store the size of AVFrame Andreas Rheinhardt
2022-02-12 11:46   ` James Almer
2022-02-12  6:45 ` Andreas Rheinhardt
2022-02-12 11:58   ` James Almer [this message]
2022-02-12 12:08 ` Michael Niedermayer
2022-02-12 12:16   ` James Almer
2022-02-13 12:08     ` Michael Niedermayer
2022-02-12 12:25 ` Paul B Mahol
2022-02-12 12:28   ` James Almer
2022-02-12 12:32     ` Paul B Mahol
2022-02-12 12:33       ` James Almer
2022-02-12 12:38         ` Paul B Mahol

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4bb99492-a8cb-7faf-e7dd-94bd7d7461b7@gmail.com \
    --to=jamrial@gmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git