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

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

[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.

 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] [PATCH 2/4] avcodec/wrapped_avframe: don't allocate an AVFrame twice

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/wrapped_avframe.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/libavcodec/wrapped_avframe.c b/libavcodec/wrapped_avframe.c
index a7834b86e8..004bd5e0e7 100644
--- a/libavcodec/wrapped_avframe.c
+++ b/libavcodec/wrapped_avframe.c
@@ -43,33 +43,32 @@ static void wrapped_avframe_release_buffer(void *unused, uint8_t *data)
 static int wrapped_avframe_encode(AVCodecContext *avctx, AVPacket *pkt,
                      const AVFrame *frame, int *got_packet)
 {
-    AVFrame *wrapped = av_frame_clone(frame);
     uint8_t *data;
-    int size = sizeof(*wrapped) + AV_INPUT_BUFFER_PADDING_SIZE;
-
-    if (!wrapped)
-        return AVERROR(ENOMEM);
+    int ret, size = sizeof(*frame) + AV_INPUT_BUFFER_PADDING_SIZE;
 
     data = av_mallocz(size);
     if (!data) {
-        av_frame_free(&wrapped);
         return AVERROR(ENOMEM);
     }
+    // Set frame defaults
+    av_frame_unref((AVFrame *)data);
 
     pkt->buf = av_buffer_create(data, size,
                                 wrapped_avframe_release_buffer, NULL,
                                 AV_BUFFER_FLAG_READONLY);
     if (!pkt->buf) {
-        av_frame_free(&wrapped);
         av_freep(&data);
         return AVERROR(ENOMEM);
     }
 
-    av_frame_move_ref((AVFrame*)data, wrapped);
-    av_frame_free(&wrapped);
+    ret = av_frame_ref((AVFrame*)data, frame);
+    if (ret < 0) {
+        av_buffer_unref(&pkt->buf);
+        return ret;
+    }
 
     pkt->data = data;
-    pkt->size = sizeof(*wrapped);
+    pkt->size = sizeof(*frame);
 
     pkt->flags |= AV_PKT_FLAG_KEY;
     *got_packet = 1;
-- 
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] [PATCH 3/4] avcodec/wrapped_avframe: stop hardcoding sizeof(AVFrame)

[James Almer]

Use instead the new internal avpriv_avframe_size constant. This will
ensure the actual size of AVFrame from the libavutil loaded at runtime
is used instead.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/wrapped_avframe.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libavcodec/wrapped_avframe.c b/libavcodec/wrapped_avframe.c
index 004bd5e0e7..543d9f85eb 100644
--- a/libavcodec/wrapped_avframe.c
+++ b/libavcodec/wrapped_avframe.c
@@ -30,6 +30,7 @@
 
 #include "libavutil/internal.h"
 #include "libavutil/frame.h"
+#include "libavutil/frame_internal.h"
 #include "libavutil/buffer.h"
 #include "libavutil/pixdesc.h"
 
@@ -44,7 +45,8 @@ static int wrapped_avframe_encode(AVCodecContext *avctx, AVPacket *pkt,
                      const AVFrame *frame, int *got_packet)
 {
     uint8_t *data;
-    int ret, size = sizeof(*frame) + AV_INPUT_BUFFER_PADDING_SIZE;
+    const size_t size = avpriv_avframe_size;
+    int ret;
 
     data = av_mallocz(size);
     if (!data) {
@@ -53,7 +55,7 @@ static int wrapped_avframe_encode(AVCodecContext *avctx, AVPacket *pkt,
     // Set frame defaults
     av_frame_unref((AVFrame *)data);
 
-    pkt->buf = av_buffer_create(data, size,
+    pkt->buf = av_buffer_create(data, size + AV_INPUT_BUFFER_PADDING_SIZE,
                                 wrapped_avframe_release_buffer, NULL,
                                 AV_BUFFER_FLAG_READONLY);
     if (!pkt->buf) {
@@ -68,7 +70,7 @@ static int wrapped_avframe_encode(AVCodecContext *avctx, AVPacket *pkt,
     }
 
     pkt->data = data;
-    pkt->size = sizeof(*frame);
+    pkt->size = size;
 
     pkt->flags |= AV_PKT_FLAG_KEY;
     *got_packet = 1;
@@ -86,7 +88,7 @@ static int wrapped_avframe_decode(AVCodecContext *avctx, void *data,
         return AVERROR(EPERM);
     }
 
-    if (pkt->size < sizeof(AVFrame))
+    if (pkt->size < avpriv_avframe_size)
         return AVERROR(EINVAL);
 
     in  = (AVFrame*)pkt->data;
-- 
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] [PATCH 4/4] avformat/vapoursynth: stop hardcoding sizeof(AVFrame)

[James Almer]

Use instead the new internal avpriv_avframe_size constant. This will
ensure the actual size of AVFrame from the libavutil loaded at runtime
is used instead.

Also add the missing padding bytes required by the AVPacket API while
at it.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/vapoursynth.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/libavformat/vapoursynth.c b/libavformat/vapoursynth.c
index 1578a6ac77..5c57f3a71c 100644
--- a/libavformat/vapoursynth.c
+++ b/libavformat/vapoursynth.c
@@ -31,6 +31,7 @@
 #include "libavutil/avassert.h"
 #include "libavutil/avstring.h"
 #include "libavutil/eval.h"
+#include "libavutil/frame_internal.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
@@ -344,6 +345,7 @@ static int read_packet_vs(AVFormatContext *s, AVPacket *pkt)
     const AVPixFmtDescriptor *desc;
     AVBufferRef *vsframe_ref = NULL;
     struct vsframe_ref_data *ref_data;
+    const size_t size = avpriv_avframe_size;
     int err = 0;
     int i;
 
@@ -382,11 +384,13 @@ static int read_packet_vs(AVFormatContext *s, AVPacket *pkt)
 
     props = vs->vsapi->getFramePropsRO(vsframe);
 
-    frame = av_frame_alloc();
+    frame = (AVFrame *)av_mallocz(size + AV_INPUT_BUFFER_PADDING_SIZE);
     if (!frame) {
         err = AVERROR(ENOMEM);
         goto end;
     }
+    // Set frame defaults
+    av_frame_unref(frame);
 
     frame->format       = st->codecpar->format;
     frame->width        = st->codecpar->width;
@@ -432,8 +436,8 @@ static int read_packet_vs(AVFormatContext *s, AVPacket *pkt)
         frame->buf[i]->size = frame->linesize[i] * plane_h;
     }
 
-    pkt->buf = av_buffer_create((uint8_t*)frame, sizeof(*frame),
-                                free_frame, NULL, 0);
+    pkt->buf = av_buffer_create((uint8_t*)frame, size + AV_INPUT_BUFFER_PADDING_SIZE,
+                                free_frame, NULL, AV_BUFFER_FLAG_READONLY);
     if (!pkt->buf) {
         err = AVERROR(ENOMEM);
         goto end;
@@ -442,7 +446,7 @@ static int read_packet_vs(AVFormatContext *s, AVPacket *pkt)
     frame = NULL; // pkt owns it now
 
     pkt->data   = pkt->buf->data;
-    pkt->size   = pkt->buf->size;
+    pkt->size   = size;
     pkt->flags |= AV_PKT_FLAG_TRUSTED;
 
     if (vs->is_cfr)
-- 
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".

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

[Andreas Rheinhardt]

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.
> 
>  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>

size_t is in stddef.h and some other headers, but not in 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 */

Missing av_export_avutil. (And aren't there systems where exporting data
is always problematic?)

- Andreas
_______________________________________________
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".

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

[James Almer]

On 2/12/2022 2:29 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.
>>
>>   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>
> 
> size_t is in stddef.h and some other headers, but not in 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 */
> 
> Missing av_export_avutil. (And aren't there systems where exporting data
> is always problematic?)

Wouldn't that mean that existing constants like av_md5_size and 
av_sha_size are not working?

> 
> - Andreas
> _______________________________________________
> 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".

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

[Andreas Rheinhardt]

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?
(Also why don't we just switch to something like what is done for
uncoded frames in libavformat/mux.c?)

>  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".

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

[James Almer]

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".

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

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1112 bytes --]

On Fri, Feb 11, 2022 at 09:12:58PM -0300, James Almer wrote:
> 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.

iam probably missing something but if the goal is to wrap AVFrame in some
other structure as a array or buffer
without the sizeof(AVFrame) cant the wraping/unwraping code be put in
libavutil ?

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

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

[James Almer]

On 2/12/2022 9:08 AM, Michael Niedermayer wrote:
> On Fri, Feb 11, 2022 at 09:12:58PM -0300, James Almer wrote:
>> 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.
> 
> iam probably missing something but if the goal is to wrap AVFrame in some
> other structure as a array or buffer
> without the sizeof(AVFrame) cant the wraping/unwraping code be put in
> libavutil ?

How would that fix the situation of setting AVPacket.size to 
sizeof(AVFrame) and AVPacket.data to an structure that big + padding 
bytes in packets returned to the caller?

> 
> thx
> 
> [...]
> 
> 
> _______________________________________________
> 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".

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

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2059 bytes --]

On Sat, Feb 12, 2022 at 09:16:36AM -0300, James Almer wrote:
> On 2/12/2022 9:08 AM, Michael Niedermayer wrote:
> > On Fri, Feb 11, 2022 at 09:12:58PM -0300, James Almer wrote:
> > > 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.
> > 
> > iam probably missing something but if the goal is to wrap AVFrame in some
> > other structure as a array or buffer
> > without the sizeof(AVFrame) cant the wraping/unwraping code be put in
> > libavutil ?
> 
> How would that fix the situation of setting AVPacket.size to sizeof(AVFrame)
> and AVPacket.data to an structure that big + padding bytes in packets
> returned to the caller?

If you had a function which turned a AVFrame into a AVBufferRef

pkt->data = buf->data;
pkt->size = buf->size;

If done carefully, this might work even independant of how that function
does it, by reference, copy or full serialization

If that direction would be pursued (possibly later), the wraping function
could have a flag that switches between zero copy reference and 
full serialization of AVFrames which could be passed accross a network 
or stored on disk and used later/elsewhere

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [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: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?


> 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".

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

[James Almer]

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).

> 
> 
>> 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".

Re: [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: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.


>
> >
> >
> >> 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".

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

[James Almer]

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".

Re: [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@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@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.
>
> 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@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".
>
_______________________________________________
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".