Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Juan De León (12019-08-19): > > > +size_t size = sizeof(AVEncodeInfoFrame) + > > sizeof(AVEncodeInfoBlock)*FFMAX((int)(nb_blocks - 1), 0); > > > > (Commenting from my phone because urgent.) > > This line do not make sense to me. Can you explain the reason for the > > cast and how you avoid overflows? > > > In AVFrameEncodeInfo the block array is initialized as blocks[1], so when > allocating memory for the array we must consider 1 block is already > allocated. > nb_blocks can be 0, if it is unsigned the subtraction wraps it around to > UINT_MAX, the cast to int is to prevent that. The cast to int does not prevent the wrap, because the wrap happens before the cast. All the cast does is cause an undefined behaviour, which will in practice yield -1, but then you multiply by a sizeof, which is unsigned. > If this is confusing, I think it's better to change it to check for > nb_blocks > 0 and subtract 1. Indeed, it is the simplest clean solution. You can do that by swapping the FFMAX and the subtraction. As it is, the FFMAX does nothing anyway. Or you can dispense with the FFMAX entirey: sizeof(info) - sizeof(block) + n * sizeof(block). It result in allocating an incomplete info structure, which I think is fine because nothing will access the missing part. In any case, you need to check that neither the multiplication nor the division overflow. And you need to do it beforehand. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
On Sat, Aug 17, 2019 at 7:00 AM Nicolas George wrote:
> > +#define AV_ENCODE_INFO_MAX_BLOCKS 33177600
> > +
> > +static int init_encode_info_data(AVEncodeInfoFrame *info, unsigned int
> nb_blocks) {
> > +info->nb_blocks = nb_blocks;
> > +info->block_size = sizeof(AVEncodeInfoBlock);
> > +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
> > +
> > +for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
> > +info->plane_q[i] = -1;
> > +
> > +return 0;
> > +}
> > +
> > +AVEncodeInfoFrame *av_encode_info_alloc(unsigned int nb_blocks)
> > +{
> > +if (nb_blocks > AV_ENCODE_INFO_MAX_BLOCKS)
> > +return NULL;
> > +
> > +//AVEncodeInfoFrame already allocates size for one element of
> AVEncodeInfoBlock
>
> > +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMAX((int)(nb_blocks - 1), 0);
>
> (Commenting from my phone because urgent.)
> This line do not make sense to me. Can you explain the reason for the
> cast and how you avoid overflows?
>
In AVFrameEncodeInfo the block array is initialized as blocks[1], so when
allocating memory for the array we must consider 1 block is already
allocated.
nb_blocks can be 0, if it is unsigned the subtraction wraps it around to
UINT_MAX, the cast to int is to prevent that.
If this is confusing, I think it's better to change it to check for
nb_blocks > 0 and subtract 1.
Thanks
On Sat, Aug 17, 2019 at 7:00 AM Nicolas George wrote:
> Juan De León (12019-08-16):
> > AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> > AV_FRAME_DATA_ENCODE_INFO.
> > The structure stores quantization index for each plane, DC/AC deltas
> > for luma and chroma planes, and an array of AVEncodeInfoBlock type
> > denoting position, size, and delta quantizer for each block in the
> > frame.
> > Can be extended to support extraction of other block information.
> >
> > Signed-off-by: Juan De León
> > ---
> > libavutil/Makefile | 2 +
> > libavutil/encode_info.c | 68 +
> > libavutil/encode_info.h | 110
> > libavutil/frame.c | 1 +
> > libavutil/frame.h | 7 +++
> > 5 files changed, 188 insertions(+)
> > create mode 100644 libavutil/encode_info.c
> > create mode 100644 libavutil/encode_info.h
> >
> > diff --git a/libavutil/Makefile b/libavutil/Makefile
> > index 57e6e3d7e8..37cfb099e9 100644
> > --- a/libavutil/Makefile
> > +++ b/libavutil/Makefile
> > @@ -24,6 +24,7 @@ HEADERS = adler32.h
> \
> >dict.h
> \
> >display.h
> \
> >downmix_info.h
> \
> > + encode_info.h
> \
> >encryption_info.h
> \
> >error.h
> \
> >eval.h
> \
> > @@ -111,6 +112,7 @@ OBJS = adler32.o
> \
> > dict.o
> \
> > display.o
> \
> > downmix_info.o
> \
> > + encode_info.o
> \
> > encryption_info.o
> \
> > error.o
> \
> > eval.o
> \
> > diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> > new file mode 100644
> > index 00..ec352a403b
> > --- /dev/null
> > +++ b/libavutil/encode_info.c
> > @@ -0,0 +1,68 @@
> > +/*
> > + * 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
> > + */
> > +
> > +#include "libavutil/encode_info.h"
> > +#include "libavutil/mem.h"
> > +
> > +// To prevent overflow, assumes max number = 1px blocks for 8k video.
> > +#define AV_ENCODE_INFO_MAX_BLOCKS 33177600
> > +
> > +static int init_encode_info_data(AVEncodeInfoFrame *info, unsigned int
> nb_blocks) {
> > +info->nb_blocks = nb_blocks;
> > +info->block_size = sizeof(AVEncodeInfoBlock);
> > +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
> > +
> > +for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
> > +info->plane_q[i] = -1;
> > +
> > +return 0;
> > +}
> > +
> > +AVEncodeInfoFrame *av_encode_info_alloc(unsigned int nb_blocks)
> > +{
> > +if (nb_blocks > AV_ENCODE_INFO_MAX_BLOCKS)
> > +return NULL;
> > +
> > +//AVEncodeInfoFrame already allocates size for one element of
> AVEncodeInfoBlock
>
> > +size_t size = sizeof(AVEncodeInfoFrame) +
>
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Juan De León (12019-08-16):
> AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> AV_FRAME_DATA_ENCODE_INFO.
> The structure stores quantization index for each plane, DC/AC deltas
> for luma and chroma planes, and an array of AVEncodeInfoBlock type
> denoting position, size, and delta quantizer for each block in the
> frame.
> Can be extended to support extraction of other block information.
>
> Signed-off-by: Juan De León
> ---
> libavutil/Makefile | 2 +
> libavutil/encode_info.c | 68 +
> libavutil/encode_info.h | 110
> libavutil/frame.c | 1 +
> libavutil/frame.h | 7 +++
> 5 files changed, 188 insertions(+)
> create mode 100644 libavutil/encode_info.c
> create mode 100644 libavutil/encode_info.h
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 57e6e3d7e8..37cfb099e9 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -24,6 +24,7 @@ HEADERS = adler32.h
> \
>dict.h\
>display.h \
>downmix_info.h\
> + encode_info.h \
>encryption_info.h \
>error.h \
>eval.h\
> @@ -111,6 +112,7 @@ OBJS = adler32.o
> \
> dict.o \
> display.o\
> downmix_info.o \
> + encode_info.o\
> encryption_info.o\
> error.o \
> eval.o \
> diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> new file mode 100644
> index 00..ec352a403b
> --- /dev/null
> +++ b/libavutil/encode_info.c
> @@ -0,0 +1,68 @@
> +/*
> + * 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
> + */
> +
> +#include "libavutil/encode_info.h"
> +#include "libavutil/mem.h"
> +
> +// To prevent overflow, assumes max number = 1px blocks for 8k video.
> +#define AV_ENCODE_INFO_MAX_BLOCKS 33177600
> +
> +static int init_encode_info_data(AVEncodeInfoFrame *info, unsigned int
> nb_blocks) {
> +info->nb_blocks = nb_blocks;
> +info->block_size = sizeof(AVEncodeInfoBlock);
> +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
> +
> +for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
> +info->plane_q[i] = -1;
> +
> +return 0;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_alloc(unsigned int nb_blocks)
> +{
> +if (nb_blocks > AV_ENCODE_INFO_MAX_BLOCKS)
> +return NULL;
> +
> +//AVEncodeInfoFrame already allocates size for one element of
> AVEncodeInfoBlock
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMAX((int)(nb_blocks - 1), 0);
(Commenting from my phone because urgent.)
This line do not make sense to me. Can you explain the reason for the
cast and how you avoid overflows?
> +AVEncodeInfoFrame *ptr = av_mallocz(size);
> +if (!ptr)
> +return NULL;
> +
> +init_encode_info_data(ptr, nb_blocks);
> +
> +return ptr;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, unsigned
> int nb_blocks)
> +{
> +if (nb_blocks > AV_ENCODE_INFO_MAX_BLOCKS)
> +return NULL;
> +
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMAX((int)(nb_blocks - 1), 0);
> +AVFrameSideData *sd = av_frame_new_side_data(frame,
> + AV_FRAME_DATA_ENCODE_INFO,
> + size);
> +if (!sd)
> +
[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
AVEncodeInfoFrame data structure to store as AVFrameSideData of type
AV_FRAME_DATA_ENCODE_INFO.
The structure stores quantization index for each plane, DC/AC deltas
for luma and chroma planes, and an array of AVEncodeInfoBlock type
denoting position, size, and delta quantizer for each block in the
frame.
Can be extended to support extraction of other block information.
Signed-off-by: Juan De León
---
libavutil/Makefile | 2 +
libavutil/encode_info.c | 68 +
libavutil/encode_info.h | 110
libavutil/frame.c | 1 +
libavutil/frame.h | 7 +++
5 files changed, 188 insertions(+)
create mode 100644 libavutil/encode_info.c
create mode 100644 libavutil/encode_info.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 57e6e3d7e8..37cfb099e9 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h
\
dict.h\
display.h \
downmix_info.h\
+ encode_info.h \
encryption_info.h \
error.h \
eval.h\
@@ -111,6 +112,7 @@ OBJS = adler32.o
\
dict.o \
display.o\
downmix_info.o \
+ encode_info.o\
encryption_info.o\
error.o \
eval.o \
diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
new file mode 100644
index 00..ec352a403b
--- /dev/null
+++ b/libavutil/encode_info.c
@@ -0,0 +1,68 @@
+/*
+ * 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
+ */
+
+#include "libavutil/encode_info.h"
+#include "libavutil/mem.h"
+
+// To prevent overflow, assumes max number = 1px blocks for 8k video.
+#define AV_ENCODE_INFO_MAX_BLOCKS 33177600
+
+static int init_encode_info_data(AVEncodeInfoFrame *info, unsigned int
nb_blocks) {
+info->nb_blocks = nb_blocks;
+info->block_size = sizeof(AVEncodeInfoBlock);
+info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
+
+for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
+info->plane_q[i] = -1;
+
+return 0;
+}
+
+AVEncodeInfoFrame *av_encode_info_alloc(unsigned int nb_blocks)
+{
+if (nb_blocks > AV_ENCODE_INFO_MAX_BLOCKS)
+return NULL;
+
+//AVEncodeInfoFrame already allocates size for one element of
AVEncodeInfoBlock
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMAX((int)(nb_blocks - 1), 0);
+AVEncodeInfoFrame *ptr = av_mallocz(size);
+if (!ptr)
+return NULL;
+
+init_encode_info_data(ptr, nb_blocks);
+
+return ptr;
+}
+
+AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, unsigned
int nb_blocks)
+{
+if (nb_blocks > AV_ENCODE_INFO_MAX_BLOCKS)
+return NULL;
+
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMAX((int)(nb_blocks - 1), 0);
+AVFrameSideData *sd = av_frame_new_side_data(frame,
+ AV_FRAME_DATA_ENCODE_INFO,
+ size);
+if (!sd)
+return NULL;
+
+memset(sd->data, 0, size);
+init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
+
+return (AVEncodeInfoFrame*)sd->data;
+}
diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
new file mode 100644
index 00..354411b9e1
--- /dev/null
+++ b/libavutil/encode_info.h
@@ -0,0 +1,110 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures (fwd)
- Forwarded message from Nicolas George - Date: Fri, 16 Aug 2019 13:55:18 +0200 From: Nicolas George To: Juan De León Subject: Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures Juan De León (12019-08-15): > I don't think it's common for size_t to wrap around. size_t has a max value, like all integer types. If the mathematical result of the multiolication or the addition is greater than that, it cannot work. Regards, -- Nicolas George - End forwarded message - ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Juan De León (12019-08-15):
> AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> AV_FRAME_DATA_ENCODE_INFO.
> The structure stores quantization index for each plane, DC/AC deltas
> for luma and chroma planes, and an array of AVEncodeInfoBlock type
> denoting position, size, and delta quantizer for each block in the
> frame.
> Can be extended to support extraction of other block information.
>
> Signed-off-by: Juan De León
> ---
> libavutil/Makefile | 2 +
> libavutil/encode_info.c | 68 +
> libavutil/encode_info.h | 110
> libavutil/frame.c | 1 +
> libavutil/frame.h | 7 +++
> 5 files changed, 188 insertions(+)
> create mode 100644 libavutil/encode_info.c
> create mode 100644 libavutil/encode_info.h
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 57e6e3d7e8..37cfb099e9 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -24,6 +24,7 @@ HEADERS = adler32.h
> \
>dict.h\
>display.h \
>downmix_info.h\
> + encode_info.h \
>encryption_info.h \
>error.h \
>eval.h\
> @@ -111,6 +112,7 @@ OBJS = adler32.o
> \
> dict.o \
> display.o\
> downmix_info.o \
> + encode_info.o\
> encryption_info.o\
> error.o \
> eval.o \
> diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> new file mode 100644
> index 00..351333cf43
> --- /dev/null
> +++ b/libavutil/encode_info.c
> @@ -0,0 +1,68 @@
> +/*
> + * 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
> + */
> +
> +#include "libavutil/encode_info.h"
> +#include "libavutil/mem.h"
> +
> +static int init_encode_info_data(AVEncodeInfoFrame *info, int nb_blocks) {
> +info->nb_blocks = nb_blocks;
> +info->block_size = sizeof(AVEncodeInfoBlock);
> +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
> +
> +for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
> +info->plane_q[i] = -1;
> +
> +return 0;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
> +{
> +AVEncodeInfoFrame *ptr;
> +//AVEncodeInfoFrame already allocates size for one element of
> AVEncodeInfoBlock
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
This can overflow.
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +ptr = av_mallocz(size);
> +if (!ptr)
> +return NULL;
> +
> +init_encode_info_data(ptr, nb_blocks);
> +
> +return ptr;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
> nb_blocks)
> +{
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +AVFrameSideData *sd = av_frame_new_side_data(frame,
> + AV_FRAME_DATA_ENCODE_INFO,
> + size);
> +if (!sd)
> +return NULL;
> +
> +memset(sd->data, 0, size);
> +init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
> +
> +return (AVEncodeInfoFrame*)sd->data;
> +}
> diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
> new file mode 100644
> index
[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
AVEncodeInfoFrame data structure to store as AVFrameSideData of type
AV_FRAME_DATA_ENCODE_INFO.
The structure stores quantization index for each plane, DC/AC deltas
for luma and chroma planes, and an array of AVEncodeInfoBlock type
denoting position, size, and delta quantizer for each block in the
frame.
Can be extended to support extraction of other block information.
Signed-off-by: Juan De León
---
libavutil/Makefile | 2 +
libavutil/encode_info.c | 68 +
libavutil/encode_info.h | 110
libavutil/frame.c | 1 +
libavutil/frame.h | 7 +++
5 files changed, 188 insertions(+)
create mode 100644 libavutil/encode_info.c
create mode 100644 libavutil/encode_info.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 57e6e3d7e8..37cfb099e9 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h
\
dict.h\
display.h \
downmix_info.h\
+ encode_info.h \
encryption_info.h \
error.h \
eval.h\
@@ -111,6 +112,7 @@ OBJS = adler32.o
\
dict.o \
display.o\
downmix_info.o \
+ encode_info.o\
encryption_info.o\
error.o \
eval.o \
diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
new file mode 100644
index 00..351333cf43
--- /dev/null
+++ b/libavutil/encode_info.c
@@ -0,0 +1,68 @@
+/*
+ * 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
+ */
+
+#include "libavutil/encode_info.h"
+#include "libavutil/mem.h"
+
+static int init_encode_info_data(AVEncodeInfoFrame *info, int nb_blocks) {
+info->nb_blocks = nb_blocks;
+info->block_size = sizeof(AVEncodeInfoBlock);
+info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
+
+for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
+info->plane_q[i] = -1;
+
+return 0;
+}
+
+AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
+{
+AVEncodeInfoFrame *ptr;
+//AVEncodeInfoFrame already allocates size for one element of
AVEncodeInfoBlock
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+ptr = av_mallocz(size);
+if (!ptr)
+return NULL;
+
+init_encode_info_data(ptr, nb_blocks);
+
+return ptr;
+}
+
+AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
nb_blocks)
+{
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+AVFrameSideData *sd = av_frame_new_side_data(frame,
+ AV_FRAME_DATA_ENCODE_INFO,
+ size);
+if (!sd)
+return NULL;
+
+memset(sd->data, 0, size);
+init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
+
+return (AVEncodeInfoFrame*)sd->data;
+}
diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
new file mode 100644
index 00..93d0e3fb56
--- /dev/null
+++ b/libavutil/encode_info.h
@@ -0,0 +1,110 @@
+/*
+ * 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;
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
On 8/15/2019 3:47 PM, Nicolas George wrote: > Alex Sukhanov (12019-08-15): >> Time is important here. Juan's internship is ending soon. Juan has been >> working with Lynne on this design. Since there is back and forth emails in >> this thread, it would be very helpful for Juan if Nicolas and Lynne get to >> some agreement, rather than have intern trying to satisfy them while having >> different views. >> Juan needs clear guidance from ffmpeg community here. Please help. > > I have not seen Lynne's arguments. And I have stated and argued my > position: either drop the test or make it an assert. All other solution > is inferior. > > Regards, Go with the assert. It's not worth wasting time discussing this further. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Aug 15, 2019, 19:47 by geo...@nsup.org: > Alex Sukhanov (12019-08-15): > >> Time is important here. Juan's internship is ending soon. Juan has been >> working with Lynne on this design. Since there is back and forth emails in >> this thread, it would be very helpful for Juan if Nicolas and Lynne get to >> some agreement, rather than have intern trying to satisfy them while having >> different views. >> Juan needs clear guidance from ffmpeg community here. Please help. >> > > I have not seen Lynne's arguments. And I have stated and argued my > position: either drop the test or make it an assert. All other solution > is inferior. > I don't really have an opinion on this, but I don't think users would even check the return value as long as they loop over the specified nb_blocks. So I'm fine with an assert, but I'd prefer to drop the test altogether. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Alex Sukhanov (12019-08-15): > Time is important here. Juan's internship is ending soon. Juan has been > working with Lynne on this design. Since there is back and forth emails in > this thread, it would be very helpful for Juan if Nicolas and Lynne get to > some agreement, rather than have intern trying to satisfy them while having > different views. > Juan needs clear guidance from ffmpeg community here. Please help. I have not seen Lynne's arguments. And I have stated and argued my position: either drop the test or make it an assert. All other solution is inferior. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
On Thu, Aug 15, 2019 at 11:07 AM Nicolas George wrote: > Juan De León (12019-08-15): > > Ping. Does anyone have any more feedback? > > If not, can anyone review this for pushing. > > Less than 24 hours feel a bit short to get impatient. > > Regards, > > -- > Nicolas George > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". Time is important here. Juan's internship is ending soon. Juan has been working with Lynne on this design. Since there is back and forth emails in this thread, it would be very helpful for Juan if Nicolas and Lynne get to some agreement, rather than have intern trying to satisfy them while having different views. Juan needs clear guidance from ffmpeg community here. Please help. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Juan De León (12019-08-14): > In that case, I believe documenting the size of the array and behaviour of > undefined indexes should be enough. Have the pointers return NULL, > and let the user handle the result, instead of stopping the execution. I disagree. Either drop the check altogether or make it an assert. A NULL return is a compromise that is worse than either. > I would prefer to discuss the actual data structure for now. This is whai I can comment on. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Juan De León (12019-08-15): > Ping. Does anyone have any more feedback? > If not, can anyone review this for pushing. Less than 24 hours feel a bit short to get impatient. Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
On Thu, Aug 15, 2019 at 7:43 PM Juan De León <
juandl-at-google@ffmpeg.org> wrote:
> On Wed, Aug 14, 2019 at 12:11 PM Juan De León wrote:
>
> > AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> > AV_FRAME_DATA_ENCODE_INFO.
> > The structure stores quantization index for each plane, DC/AC deltas
> > for luma and chroma planes, and an array of AVEncodeInfoBlock type
> > denoting position, size, and delta quantizer for each block in the
> > frame.
> > Can be extended to support extraction of other block information.
> >
> > Signed-off-by: Juan De León
> > ---
> > libavutil/Makefile | 2 +
> > libavutil/encode_info.c | 68
> > libavutil/encode_info.h | 112
> > libavutil/frame.c | 1 +
> > libavutil/frame.h | 7 +++
> > 5 files changed, 190 insertions(+)
> > create mode 100644 libavutil/encode_info.c
> > create mode 100644 libavutil/encode_info.h
> >
> > diff --git a/libavutil/Makefile b/libavutil/Makefile
> > index 57e6e3d7e8..37cfb099e9 100644
> > --- a/libavutil/Makefile
> > +++ b/libavutil/Makefile
> > @@ -24,6 +24,7 @@ HEADERS = adler32.h
> >\
> >dict.h
> \
> >display.h
> \
> >downmix_info.h
> \
> > + encode_info.h
> \
> >encryption_info.h
> \
> >error.h
> \
> >eval.h
> \
> > @@ -111,6 +112,7 @@ OBJS = adler32.o
> > \
> > dict.o
> \
> > display.o
> \
> > downmix_info.o
> \
> > + encode_info.o
> \
> > encryption_info.o
> \
> > error.o
> \
> > eval.o
> \
> > diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> > new file mode 100644
> > index 00..351333cf43
> > --- /dev/null
> > +++ b/libavutil/encode_info.c
> > @@ -0,0 +1,68 @@
> > +/*
> > + * 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
> > + */
> > +
> > +#include "libavutil/encode_info.h"
> > +#include "libavutil/mem.h"
> > +
> > +static int init_encode_info_data(AVEncodeInfoFrame *info, int
> nb_blocks) {
> > +info->nb_blocks = nb_blocks;
> > +info->block_size = sizeof(AVEncodeInfoBlock);
> > +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
> > +
> > +for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
> > +info->plane_q[i] = -1;
> > +
> > +return 0;
> > +}
> > +
> > +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
> > +{
> > +AVEncodeInfoFrame *ptr;
> > +//AVEncodeInfoFrame already allocates size for one element of
> > AVEncodeInfoBlock
> > +size_t size = sizeof(AVEncodeInfoFrame) +
> > sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
> > +
> > +if (nb_blocks < 0 || size >= INT_MAX)
> > +return NULL;
> > +
> > +ptr = av_mallocz(size);
> > +if (!ptr)
> > +return NULL;
> > +
> > +init_encode_info_data(ptr, nb_blocks);
> > +
> > +return ptr;
> > +}
> > +
> > +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
> > nb_blocks)
> > +{
> > +size_t size = sizeof(AVEncodeInfoFrame) +
> > sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
> > +
> > +if (nb_blocks < 0 || size >= INT_MAX)
> > +return NULL;
> > +
> > +AVFrameSideData *sd = av_frame_new_side_data(frame,
> > +
> > AV_FRAME_DATA_ENCODE_INFO,
> > + size);
> > +if (!sd)
> > +return NULL;
> > +
> > +memset(sd->data, 0, size);
> > +init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
> > +
> > +return (AVEncodeInfoFrame*)sd->data;
> > +}
> > diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
> > new file mode 100644
> > index 00..ae0a6563dc
> > --- /dev/null
> > +++ b/libavutil/encode_info.h
> > @@ -0,0 +1,112 @@
> > +/*
> > + * 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
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
On Wed, Aug 14, 2019 at 12:11 PM Juan De León wrote:
> AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> AV_FRAME_DATA_ENCODE_INFO.
> The structure stores quantization index for each plane, DC/AC deltas
> for luma and chroma planes, and an array of AVEncodeInfoBlock type
> denoting position, size, and delta quantizer for each block in the
> frame.
> Can be extended to support extraction of other block information.
>
> Signed-off-by: Juan De León
> ---
> libavutil/Makefile | 2 +
> libavutil/encode_info.c | 68
> libavutil/encode_info.h | 112
> libavutil/frame.c | 1 +
> libavutil/frame.h | 7 +++
> 5 files changed, 190 insertions(+)
> create mode 100644 libavutil/encode_info.c
> create mode 100644 libavutil/encode_info.h
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 57e6e3d7e8..37cfb099e9 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -24,6 +24,7 @@ HEADERS = adler32.h
>\
>dict.h\
>display.h \
>downmix_info.h\
> + encode_info.h \
>encryption_info.h \
>error.h \
>eval.h\
> @@ -111,6 +112,7 @@ OBJS = adler32.o
> \
> dict.o \
> display.o\
> downmix_info.o \
> + encode_info.o\
> encryption_info.o\
> error.o \
> eval.o \
> diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> new file mode 100644
> index 00..351333cf43
> --- /dev/null
> +++ b/libavutil/encode_info.c
> @@ -0,0 +1,68 @@
> +/*
> + * 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
> + */
> +
> +#include "libavutil/encode_info.h"
> +#include "libavutil/mem.h"
> +
> +static int init_encode_info_data(AVEncodeInfoFrame *info, int nb_blocks) {
> +info->nb_blocks = nb_blocks;
> +info->block_size = sizeof(AVEncodeInfoBlock);
> +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
> +
> +for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
> +info->plane_q[i] = -1;
> +
> +return 0;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
> +{
> +AVEncodeInfoFrame *ptr;
> +//AVEncodeInfoFrame already allocates size for one element of
> AVEncodeInfoBlock
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +ptr = av_mallocz(size);
> +if (!ptr)
> +return NULL;
> +
> +init_encode_info_data(ptr, nb_blocks);
> +
> +return ptr;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
> nb_blocks)
> +{
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +AVFrameSideData *sd = av_frame_new_side_data(frame,
> +
> AV_FRAME_DATA_ENCODE_INFO,
> + size);
> +if (!sd)
> +return NULL;
> +
> +memset(sd->data, 0, size);
> +init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
> +
> +return (AVEncodeInfoFrame*)sd->data;
> +}
> diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
> new file mode 100644
> index 00..ae0a6563dc
> --- /dev/null
> +++ b/libavutil/encode_info.h
> @@ -0,0 +1,112 @@
> +/*
> + * This file is part of FFmpeg.
>
[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
AVEncodeInfoFrame data structure to store as AVFrameSideData of type
AV_FRAME_DATA_ENCODE_INFO.
The structure stores quantization index for each plane, DC/AC deltas
for luma and chroma planes, and an array of AVEncodeInfoBlock type
denoting position, size, and delta quantizer for each block in the
frame.
Can be extended to support extraction of other block information.
Signed-off-by: Juan De León
---
libavutil/Makefile | 2 +
libavutil/encode_info.c | 68
libavutil/encode_info.h | 112
libavutil/frame.c | 1 +
libavutil/frame.h | 7 +++
5 files changed, 190 insertions(+)
create mode 100644 libavutil/encode_info.c
create mode 100644 libavutil/encode_info.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 57e6e3d7e8..37cfb099e9 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h
\
dict.h\
display.h \
downmix_info.h\
+ encode_info.h \
encryption_info.h \
error.h \
eval.h\
@@ -111,6 +112,7 @@ OBJS = adler32.o
\
dict.o \
display.o\
downmix_info.o \
+ encode_info.o\
encryption_info.o\
error.o \
eval.o \
diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
new file mode 100644
index 00..351333cf43
--- /dev/null
+++ b/libavutil/encode_info.c
@@ -0,0 +1,68 @@
+/*
+ * 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
+ */
+
+#include "libavutil/encode_info.h"
+#include "libavutil/mem.h"
+
+static int init_encode_info_data(AVEncodeInfoFrame *info, int nb_blocks) {
+info->nb_blocks = nb_blocks;
+info->block_size = sizeof(AVEncodeInfoBlock);
+info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
+
+for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
+info->plane_q[i] = -1;
+
+return 0;
+}
+
+AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
+{
+AVEncodeInfoFrame *ptr;
+//AVEncodeInfoFrame already allocates size for one element of
AVEncodeInfoBlock
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+ptr = av_mallocz(size);
+if (!ptr)
+return NULL;
+
+init_encode_info_data(ptr, nb_blocks);
+
+return ptr;
+}
+
+AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
nb_blocks)
+{
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMAX(nb_blocks - 1, 0);
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+AVFrameSideData *sd = av_frame_new_side_data(frame,
+ AV_FRAME_DATA_ENCODE_INFO,
+ size);
+if (!sd)
+return NULL;
+
+memset(sd->data, 0, size);
+init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
+
+return (AVEncodeInfoFrame*)sd->data;
+}
diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
new file mode 100644
index 00..ae0a6563dc
--- /dev/null
+++ b/libavutil/encode_info.h
@@ -0,0 +1,112 @@
+/*
+ * 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;
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
On Wed, Aug 14, 2019 at 12:10 AM Nicolas George wrote: > James Almer (12019-08-13): > > I'm fairly sure this was discussed before, but invalid arguments > > shouldn't crash an user's application. They even have their own > > standardized errno value for this purpose. > > asserts() are to catch bugs in our code, not in theirs. Returning a NULL > > pointer is the preferred behavior. > > I do not agree. Asserts are to catch all bugs that can be catched > statically. There is no sense in making some bugs harder to catch just > because they involve separate developers. > > Nobody can predict whether a disk will make a I/O error, whether there > will be enough memory, etc., that kind of error MUST be handled > gracefully. > > On the other hand, it is easy to make sure that a buffer given to read() > is not NULL, and therefore it is very acceptable to just crash when that > happens. > > EINVAL is for cases where the acceptable value cannot be easily > predicted by the caller. For example, setting an unsupported sample rate > for the sound device: the caller could check in advance, but that would > be cumbersome. > > Now, please tell me, according to you, "idx is not smaller than > nb_blocks", is it more akin to a disk I/O error, a NULL buffer or an > invalid sample rate? > > Another argument: > > Instead of providing utility code, we could just document the > arithmetic. In that case, the application would have code that says, in > essence: "block = info + offset + idx * block_size". No check. What > would happen if idx was too big? Not a graceful error: a crash, or > worse. > > The assert mimics that, in a more convenient way since it gives the > reason and line number, and does not allow the bug to devolve into > something more serious than a crash. > > Regards, > > -- > Nicolas George > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". In that case, I believe documenting the size of the array and behaviour of undefined indexes should be enough. Have the pointers return NULL, and let the user handle the result, instead of stopping the execution. I would prefer to discuss the actual data structure for now. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
James Almer (12019-08-13): > I'm fairly sure this was discussed before, but invalid arguments > shouldn't crash an user's application. They even have their own > standardized errno value for this purpose. > asserts() are to catch bugs in our code, not in theirs. Returning a NULL > pointer is the preferred behavior. I do not agree. Asserts are to catch all bugs that can be catched statically. There is no sense in making some bugs harder to catch just because they involve separate developers. Nobody can predict whether a disk will make a I/O error, whether there will be enough memory, etc., that kind of error MUST be handled gracefully. On the other hand, it is easy to make sure that a buffer given to read() is not NULL, and therefore it is very acceptable to just crash when that happens. EINVAL is for cases where the acceptable value cannot be easily predicted by the caller. For example, setting an unsupported sample rate for the sound device: the caller could check in advance, but that would be cumbersome. Now, please tell me, according to you, "idx is not smaller than nb_blocks", is it more akin to a disk I/O error, a NULL buffer or an invalid sample rate? Another argument: Instead of providing utility code, we could just document the arithmetic. In that case, the application would have code that says, in essence: "block = info + offset + idx * block_size". No check. What would happen if idx was too big? Not a graceful error: a crash, or worse. The assert mimics that, in a more convenient way since it gives the reason and line number, and does not allow the bug to devolve into something more serious than a crash. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
On Tue, Aug 13, 2019 at 4:49 PM James Almer wrote: > I'm fairly sure this was discussed before, but invalid arguments > shouldn't crash an user's application. They even have their own > standardized errno value for this purpose. > asserts() are to catch bugs in our code, not in theirs. Returning a NULL > pointer is the preferred behavior. > Thank you for the feedback, I will update the patch accordingly. Also I noticed I was using FFMIN instead of FFMAX here: size_t size = sizeof(AVEncodeInfoFrame) + sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks - 1, 0); If anyone has any more feedback or wants to discuss the patch I'll also be available in the IRC channel. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
On 8/13/2019 7:29 PM, Nicolas George wrote: > Juan De León (12019-08-13): >> The array is there so that the structure isn't opaque, it should be >> accessed with the function. > > I realize you need it, but not for the reason you say. It is needed for > alignment: if blocks needs more alignment than info, info+sizeof(info) > is not a valid pointer for blocks. > +if (!info || idx >= info->nb_blocks || idx < 0) +return NULL; >>> How valid is it for applications to call with idx outside the range? >> They shouldn't but I figure it's better to return NULL than to get >> undefined behaviour. > > In that case, I think is is better practice to document: > > Index must be between 0 and nb_blocks > > and check with an assert, forcing the application programmer fix their > code immediately. > > Most code will likely use idx from a loop counter, where it cannot be > outside the range, and for the few other cases, the caller can do the > check if necessary. > > Also, make the fields that cannot be negative unsigned, and you can drop > the <0 test. > > Regards, I'm fairly sure this was discussed before, but invalid arguments shouldn't crash an user's application. They even have their own standardized errno value for this purpose. asserts() are to catch bugs in our code, not in theirs. Returning a NULL pointer is the preferred behavior. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
AVEncodeInfoFrame data structure to store as AVFrameSideData of type
AV_FRAME_DATA_ENCODE_INFO.
The structure stores quantization index for each plane, DC/AC deltas
for luma and chroma planes, and an array of AVEncodeInfoBlock type
denoting position, size, and delta quantizer for each block in the
frame.
Can be extended to support extraction of other block information.
Signed-off-by: Juan De León
---
libavutil/Makefile | 2 +
libavutil/encode_info.c | 68 +
libavutil/encode_info.h | 110
libavutil/frame.c | 1 +
libavutil/frame.h | 7 +++
5 files changed, 188 insertions(+)
create mode 100644 libavutil/encode_info.c
create mode 100644 libavutil/encode_info.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 57e6e3d7e8..37cfb099e9 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h
\
dict.h\
display.h \
downmix_info.h\
+ encode_info.h \
encryption_info.h \
error.h \
eval.h\
@@ -111,6 +112,7 @@ OBJS = adler32.o
\
dict.o \
display.o\
downmix_info.o \
+ encode_info.o\
encryption_info.o\
error.o \
eval.o \
diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
new file mode 100644
index 00..631934efc2
--- /dev/null
+++ b/libavutil/encode_info.c
@@ -0,0 +1,68 @@
+/*
+ * 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
+ */
+
+#include "libavutil/encode_info.h"
+#include "libavutil/mem.h"
+
+static int init_encode_info_data(AVEncodeInfoFrame *info, int nb_blocks) {
+info->nb_blocks = nb_blocks;
+info->block_size = sizeof(AVEncodeInfoBlock);
+info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
+
+for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
+info->plane_q[i] = -1;
+
+return 0;
+}
+
+AVEncodeInfoFrame *av_encode_info_alloc(unsigned nb_blocks)
+{
+AVEncodeInfoFrame *ptr;
+//AVEncodeInfoFrame already allocates size for one element of
AVEncodeInfoBlock
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks - 1, 0);
+
+if (size >= INT_MAX)
+return NULL;
+
+ptr = av_mallocz(size);
+if (!ptr)
+return NULL;
+
+init_encode_info_data(ptr, nb_blocks);
+
+return ptr;
+}
+
+AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, unsigned
nb_blocks)
+{
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks - 1, 0);
+
+if (size >= INT_MAX)
+return NULL;
+
+AVFrameSideData *sd = av_frame_new_side_data(frame,
+ AV_FRAME_DATA_ENCODE_INFO,
+ size);
+if (!sd)
+return NULL;
+
+memset(sd->data, 0, size);
+init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
+
+return (AVEncodeInfoFrame*)sd->data;
+}
diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
new file mode 100644
index 00..8408deb382
--- /dev/null
+++ b/libavutil/encode_info.h
@@ -0,0 +1,110 @@
+/*
+ * 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
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Juan De León (12019-08-13): > The array is there so that the structure isn't opaque, it should be > accessed with the function. I realize you need it, but not for the reason you say. It is needed for alignment: if blocks needs more alignment than info, info+sizeof(info) is not a valid pointer for blocks. > > > +if (!info || idx >= info->nb_blocks || idx < 0) > > > +return NULL; > > How valid is it for applications to call with idx outside the range? > They shouldn't but I figure it's better to return NULL than to get > undefined behaviour. In that case, I think is is better practice to document: Index must be between 0 and nb_blocks and check with an assert, forcing the application programmer fix their code immediately. Most code will likely use idx from a loop counter, where it cannot be outside the range, and for the few other cases, the caller can do the check if necessary. Also, make the fields that cannot be negative unsigned, and you can drop the <0 test. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
On Tue, Aug 13, 2019 at 2:49 PM Nicolas George wrote: > > +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks); > > You can use sizeof(AVEncodeInfoFrame) and dispense with the blocks final > array entirely. > The array is there so that the structure isn't opaque, it should be accessed with the function. > > +if (!info || idx >= info->nb_blocks || idx < 0) > > +return NULL; > > How valid is it for applications to call with idx outside the range? They shouldn't but I figure it's better to return NULL than to get undefined behaviour. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Juan De León (12019-08-13):
> AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> AV_FRAME_DATA_ENCODE_INFO.
> The structure stores quantization index for each plane, DC/AC deltas
> for luma and chroma planes, and an array of AVEncodeInfoBlock type
> denoting position, size, and delta quantizer for each block in the
> frame.
> Can be extended to support extraction of other block information.
>
> Signed-off-by: Juan De León
> ---
> libavutil/Makefile | 2 +
> libavutil/encode_info.c | 68 +
> libavutil/encode_info.h | 110
> libavutil/frame.c | 1 +
> libavutil/frame.h | 7 +++
> 5 files changed, 188 insertions(+)
> create mode 100644 libavutil/encode_info.c
> create mode 100644 libavutil/encode_info.h
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 57e6e3d7e8..37cfb099e9 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -24,6 +24,7 @@ HEADERS = adler32.h
> \
>dict.h\
>display.h \
>downmix_info.h\
> + encode_info.h \
>encryption_info.h \
>error.h \
>eval.h\
> @@ -111,6 +112,7 @@ OBJS = adler32.o
> \
> dict.o \
> display.o\
> downmix_info.o \
> + encode_info.o\
> encryption_info.o\
> error.o \
> eval.o \
> diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> new file mode 100644
> index 00..ffc43f2c19
> --- /dev/null
> +++ b/libavutil/encode_info.c
> @@ -0,0 +1,68 @@
> +/*
> + * 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
> + */
> +
> +#include "libavutil/encode_info.h"
> +#include "libavutil/mem.h"
> +
> +static int init_encode_info_data(AVEncodeInfoFrame *info, int nb_blocks) {
> +info->nb_blocks = nb_blocks;
> +info->block_size = sizeof(AVEncodeInfoBlock);
> +info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
You can use sizeof(AVEncodeInfoFrame) and dispense with the blocks final
array entirely.
> +
> +for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
> +info->plane_q[i] = -1;
> +
> +return 0;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
> +{
> +AVEncodeInfoFrame *ptr;
> +//AVEncodeInfoFrame already allocates size for one element of
> AVEncodeInfoBlock
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks - 1, 0);
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +ptr = av_mallocz(size);
> +if (!ptr)
> +return NULL;
> +
> +init_encode_info_data(ptr, nb_blocks);
> +
> +return ptr;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
> nb_blocks)
> +{
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks - 1, 0);
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +AVFrameSideData *sd = av_frame_new_side_data(frame,
> + AV_FRAME_DATA_ENCODE_INFO,
> + size);
> +if (!sd)
> +return NULL;
> +
> +memset(sd->data, 0, size);
> +init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
> +
> +return (AVEncodeInfoFrame*)sd->data;
> +}
> diff --git
[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
AVEncodeInfoFrame data structure to store as AVFrameSideData of type
AV_FRAME_DATA_ENCODE_INFO.
The structure stores quantization index for each plane, DC/AC deltas
for luma and chroma planes, and an array of AVEncodeInfoBlock type
denoting position, size, and delta quantizer for each block in the
frame.
Can be extended to support extraction of other block information.
Signed-off-by: Juan De León
---
libavutil/Makefile | 2 +
libavutil/encode_info.c | 68 +
libavutil/encode_info.h | 110
libavutil/frame.c | 1 +
libavutil/frame.h | 7 +++
5 files changed, 188 insertions(+)
create mode 100644 libavutil/encode_info.c
create mode 100644 libavutil/encode_info.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 57e6e3d7e8..37cfb099e9 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h
\
dict.h\
display.h \
downmix_info.h\
+ encode_info.h \
encryption_info.h \
error.h \
eval.h\
@@ -111,6 +112,7 @@ OBJS = adler32.o
\
dict.o \
display.o\
downmix_info.o \
+ encode_info.o\
encryption_info.o\
error.o \
eval.o \
diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
new file mode 100644
index 00..ffc43f2c19
--- /dev/null
+++ b/libavutil/encode_info.c
@@ -0,0 +1,68 @@
+/*
+ * 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
+ */
+
+#include "libavutil/encode_info.h"
+#include "libavutil/mem.h"
+
+static int init_encode_info_data(AVEncodeInfoFrame *info, int nb_blocks) {
+info->nb_blocks = nb_blocks;
+info->block_size = sizeof(AVEncodeInfoBlock);
+info->blocks_offset = offsetof(AVEncodeInfoFrame, blocks);
+
+for(int i = 0; i < AV_NUM_DATA_POINTERS; i++)
+info->plane_q[i] = -1;
+
+return 0;
+}
+
+AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
+{
+AVEncodeInfoFrame *ptr;
+//AVEncodeInfoFrame already allocates size for one element of
AVEncodeInfoBlock
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks - 1, 0);
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+ptr = av_mallocz(size);
+if (!ptr)
+return NULL;
+
+init_encode_info_data(ptr, nb_blocks);
+
+return ptr;
+}
+
+AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
nb_blocks)
+{
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks - 1, 0);
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+AVFrameSideData *sd = av_frame_new_side_data(frame,
+ AV_FRAME_DATA_ENCODE_INFO,
+ size);
+if (!sd)
+return NULL;
+
+memset(sd->data, 0, size);
+init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
+
+return (AVEncodeInfoFrame*)sd->data;
+}
diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
new file mode 100644
index 00..864e42bdcf
--- /dev/null
+++ b/libavutil/encode_info.h
@@ -0,0 +1,110 @@
+/*
+ * 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;
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
On Mon, Aug 12, 2019 at 11:25:59AM -0700, Juan De León wrote: > Pinging, > Any other opinions? About adding a reserved[128] ? i think storing the size somewhere is probably better (less memory, and not having an arbitrary limit which could theoretically be hit one day) thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Democracy is the form of government in which you can choose your dictator signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Pinging,
Any other opinions?
On Sat, Aug 10, 2019 at 2:22 AM Nicolas George wrote:
> Lynne (12019-08-10):
> > >> +typedef struct AVEncodeInfoBlock{
> > >> +/**
> > >> + * Distance in luma pixels from the top-left corner of the
> visible frame
> > >> + * to the top-left corner of the block.
> > >> + * Can be negative if top/right padding is present on the coded
> frame.
> > >> + */
> > >> +int src_x, src_y;
> > >> +/**
> > >> + * Width and height of the block in luma pixels
> > >> + */
> > >> +int w, h;
> > >> +/**
> > >> + * Delta quantization index for the block
> > >> + */
> > >> +int delta_q;
> > >> +
> > >>
> > >> +uint8_t reserved[128];
> > >>
> > > What are these (this one and the one below) reserved fields for?
> >
> > For future extensions without breaking the API. Things like block type,
> prediction type, motion vectors, references, etc.
>
> I suspected as much. But remember that setting the size of reserved
> after fields are added will be very tricky: it requires taking into
> account alignment and padding in the structure.
>
> I think something like that might be easier to manage (and also use less
> memory right now):
>
> typedef struct AVEncodeInfoFrame {
> ...
> size_t blocks_offset;
> size_t block_size;
> }
>
> static inline AVEncodeInfoBlock *
> av_encode_info_block(AVEncodeInfoFrame *info, unsigned idx)
> {
> return (AVEncodeInfoBlock *)
>((char *)info + info->blocks_offset +
> idx * info->block_size);
> }
>
> static inline AVEncodeInfoBlock *
> av_encode_info_block_next(AVEncodeInfoFrame *info, AVEncodeInfoBlock
> *block)
> {
> return (AVEncodeInfoBlock *)
>((char *)block + info->block_size);
> }
>
> Regards,
>
> --
> Nicolas George
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Lynne (12019-08-10):
> >> +typedef struct AVEncodeInfoBlock{
> >> +/**
> >> + * Distance in luma pixels from the top-left corner of the visible
> >> frame
> >> + * to the top-left corner of the block.
> >> + * Can be negative if top/right padding is present on the coded frame.
> >> + */
> >> +int src_x, src_y;
> >> +/**
> >> + * Width and height of the block in luma pixels
> >> + */
> >> +int w, h;
> >> +/**
> >> + * Delta quantization index for the block
> >> + */
> >> +int delta_q;
> >> +
> >>
> >> +uint8_t reserved[128];
> >>
> > What are these (this one and the one below) reserved fields for?
>
> For future extensions without breaking the API. Things like block type,
> prediction type, motion vectors, references, etc.
I suspected as much. But remember that setting the size of reserved
after fields are added will be very tricky: it requires taking into
account alignment and padding in the structure.
I think something like that might be easier to manage (and also use less
memory right now):
typedef struct AVEncodeInfoFrame {
...
size_t blocks_offset;
size_t block_size;
}
static inline AVEncodeInfoBlock *
av_encode_info_block(AVEncodeInfoFrame *info, unsigned idx)
{
return (AVEncodeInfoBlock *)
((char *)info + info->blocks_offset +
idx * info->block_size);
}
static inline AVEncodeInfoBlock *
av_encode_info_block_next(AVEncodeInfoFrame *info, AVEncodeInfoBlock *block)
{
return (AVEncodeInfoBlock *)
((char *)block + info->block_size);
}
Regards,
--
Nicolas George
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Aug 9, 2019, 22:54 by geo...@nsup.org:
> Juan De León (12019-08-09):
>
>> AVEncodeInfoFrame data structure to store as AVFrameSideData of type
>> AV_FRAME_DATA_ENCODE_INFO.
>> The structure stores quantization index for each plane, DC/AC deltas
>> for luma and chroma planes, and an array of AVEncodeInfoBlock type
>> denoting position, size, and delta quantizer for each block in the
>> frame.
>> Can be extended to support extraction of other block information.
>>
>> Signed-off-by: Juan De León
>> ---
>> libavutil/Makefile | 2 +
>> libavutil/encode_info.c | 68 +++
>> libavutil/encode_info.h | 90 +
>> libavutil/frame.c | 1 +
>> libavutil/frame.h | 7
>> 5 files changed, 168 insertions(+)
>> create mode 100644 libavutil/encode_info.c
>> create mode 100644 libavutil/encode_info.h
>>
>> diff --git a/libavutil/Makefile b/libavutil/Makefile
>> index 57e6e3d7e8..37cfb099e9 100644
>> --- a/libavutil/Makefile
>> +++ b/libavutil/Makefile
>> @@ -24,6 +24,7 @@ HEADERS = adler32.h
>> \
>> dict.h\
>> display.h \
>> downmix_info.h\
>> + encode_info.h \
>> encryption_info.h \
>> error.h \
>> eval.h\
>> @@ -111,6 +112,7 @@ OBJS = adler32.o
>>\
>> dict.o \
>> display.o\
>> downmix_info.o \
>> + encode_info.o\
>> encryption_info.o\
>> error.o \
>> eval.o \
>> diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
>> new file mode 100644
>> index 00..6d832b2e36
>> --- /dev/null
>> +++ b/libavutil/encode_info.c
>> @@ -0,0 +1,68 @@
>> +/*
>> + * 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
>> + */
>> +
>> +#include "libavutil/encode_info.h"
>> +#include "libavutil/mem.h"
>> +
>> +static int init_encode_info_data(AVEncodeInfoFrame *ptr, int nb_blocks) {
>> +ptr->nb_blocks = nb_blocks;
>> +ptr->dc_q = ptr->ac_q = -1;
>> +ptr->dc_chroma_q = ptr->ac_chroma_q = -1;
>> +
>> +for(int i=0;i> +ptr->plane_q[i] = -1;
>> +
>> +return 0;
>> +}
>> +
>> +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
>> +{
>> +AVEncodeInfoFrame *ptr;
>> +//AVEncodeInfoFrame already allocates size for one element of
>> AVEncodeInfoBlock
>> +size_t size = sizeof(AVEncodeInfoFrame) +
>> sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks-1, 0);
>> +
>> +if (nb_blocks < 0 || size >= INT_MAX)
>> +return NULL;
>> +
>> +ptr = av_mallocz(size);
>> +if (!ptr)
>> +return NULL;
>> +
>> +init_encode_info_data(ptr, nb_blocks);
>> +
>> +return ptr;
>> +}
>> +
>> +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
>> nb_blocks)
>> +{
>> +size_t size = sizeof(AVEncodeInfoFrame) +
>> sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks-1, 0);
>> +
>> +if (nb_blocks < 0 || size >= INT_MAX)
>> +return NULL;
>> +
>> +AVFrameSideData *sd = av_frame_new_side_data(frame,
>> + AV_FRAME_DATA_ENCODE_INFO,
>> + size);
>> +if (!sd)
>> +return NULL;
>> +
>> +memset(sd->data, 0, size);
>> +init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
>> +
>> +return (AVEncodeInfoFrame*)sd->data;
>> +}
>> diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
>> new file mode 100644
>> index 00..f4175b43c9
>> --- /dev/null
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Juan De León (12019-08-09):
> AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> AV_FRAME_DATA_ENCODE_INFO.
> The structure stores quantization index for each plane, DC/AC deltas
> for luma and chroma planes, and an array of AVEncodeInfoBlock type
> denoting position, size, and delta quantizer for each block in the
> frame.
> Can be extended to support extraction of other block information.
>
> Signed-off-by: Juan De León
> ---
> libavutil/Makefile | 2 +
> libavutil/encode_info.c | 68 +++
> libavutil/encode_info.h | 90 +
> libavutil/frame.c | 1 +
> libavutil/frame.h | 7
> 5 files changed, 168 insertions(+)
> create mode 100644 libavutil/encode_info.c
> create mode 100644 libavutil/encode_info.h
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 57e6e3d7e8..37cfb099e9 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -24,6 +24,7 @@ HEADERS = adler32.h
> \
>dict.h\
>display.h \
>downmix_info.h\
> + encode_info.h \
>encryption_info.h \
>error.h \
>eval.h\
> @@ -111,6 +112,7 @@ OBJS = adler32.o
> \
> dict.o \
> display.o\
> downmix_info.o \
> + encode_info.o\
> encryption_info.o\
> error.o \
> eval.o \
> diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> new file mode 100644
> index 00..6d832b2e36
> --- /dev/null
> +++ b/libavutil/encode_info.c
> @@ -0,0 +1,68 @@
> +/*
> + * 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
> + */
> +
> +#include "libavutil/encode_info.h"
> +#include "libavutil/mem.h"
> +
> +static int init_encode_info_data(AVEncodeInfoFrame *ptr, int nb_blocks) {
> +ptr->nb_blocks = nb_blocks;
> +ptr->dc_q = ptr->ac_q = -1;
> +ptr->dc_chroma_q = ptr->ac_chroma_q = -1;
> +
> +for(int i=0;i +ptr->plane_q[i] = -1;
> +
> +return 0;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
> +{
> +AVEncodeInfoFrame *ptr;
> +//AVEncodeInfoFrame already allocates size for one element of
> AVEncodeInfoBlock
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks-1, 0);
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +ptr = av_mallocz(size);
> +if (!ptr)
> +return NULL;
> +
> +init_encode_info_data(ptr, nb_blocks);
> +
> +return ptr;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
> nb_blocks)
> +{
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks-1, 0);
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +AVFrameSideData *sd = av_frame_new_side_data(frame,
> + AV_FRAME_DATA_ENCODE_INFO,
> + size);
> +if (!sd)
> +return NULL;
> +
> +memset(sd->data, 0, size);
> +init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
> +
> +return (AVEncodeInfoFrame*)sd->data;
> +}
> diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
> new file mode 100644
> index 00..f4175b43c9
> --- /dev/null
> +++ b/libavutil/encode_info.h
> @@ -0,0 +1,90 @@
> +/*
> + *
[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
AVEncodeInfoFrame data structure to store as AVFrameSideData of type
AV_FRAME_DATA_ENCODE_INFO.
The structure stores quantization index for each plane, DC/AC deltas
for luma and chroma planes, and an array of AVEncodeInfoBlock type
denoting position, size, and delta quantizer for each block in the
frame.
Can be extended to support extraction of other block information.
Signed-off-by: Juan De León
---
libavutil/Makefile | 2 +
libavutil/encode_info.c | 68 +++
libavutil/encode_info.h | 90 +
libavutil/frame.c | 1 +
libavutil/frame.h | 7
5 files changed, 168 insertions(+)
create mode 100644 libavutil/encode_info.c
create mode 100644 libavutil/encode_info.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 57e6e3d7e8..37cfb099e9 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h
\
dict.h\
display.h \
downmix_info.h\
+ encode_info.h \
encryption_info.h \
error.h \
eval.h\
@@ -111,6 +112,7 @@ OBJS = adler32.o
\
dict.o \
display.o\
downmix_info.o \
+ encode_info.o\
encryption_info.o\
error.o \
eval.o \
diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
new file mode 100644
index 00..6d832b2e36
--- /dev/null
+++ b/libavutil/encode_info.c
@@ -0,0 +1,68 @@
+/*
+ * 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
+ */
+
+#include "libavutil/encode_info.h"
+#include "libavutil/mem.h"
+
+static int init_encode_info_data(AVEncodeInfoFrame *ptr, int nb_blocks) {
+ptr->nb_blocks = nb_blocks;
+ptr->dc_q = ptr->ac_q = -1;
+ptr->dc_chroma_q = ptr->ac_chroma_q = -1;
+
+for(int i=0;iplane_q[i] = -1;
+
+return 0;
+}
+
+AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
+{
+AVEncodeInfoFrame *ptr;
+//AVEncodeInfoFrame already allocates size for one element of
AVEncodeInfoBlock
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks-1, 0);
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+ptr = av_mallocz(size);
+if (!ptr)
+return NULL;
+
+init_encode_info_data(ptr, nb_blocks);
+
+return ptr;
+}
+
+AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
nb_blocks)
+{
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*FFMIN(nb_blocks-1, 0);
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+AVFrameSideData *sd = av_frame_new_side_data(frame,
+ AV_FRAME_DATA_ENCODE_INFO,
+ size);
+if (!sd)
+return NULL;
+
+memset(sd->data, 0, size);
+init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
+
+return (AVEncodeInfoFrame*)sd->data;
+}
diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
new file mode 100644
index 00..f4175b43c9
--- /dev/null
+++ b/libavutil/encode_info.h
@@ -0,0 +1,90 @@
+/*
+ * 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.
+ *
+ *
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures and AV_FRAME_DATA_ENCODE_INFO AVFrameSideDataType
On 8/9/2019 3:28 PM, Nicolas George wrote: > James Almer (12019-08-09): >> Or just a pointer that points to the first byte after itself. > > The pointer takes places by itself. And it prevents the structure from > being copied flatly, which IIRC is forbidden with side data. Yeah, you're right, make_writable() and similar functions would break it. > > By the way, the lines of the commit message are too long. > > Regards, > > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures and AV_FRAME_DATA_ENCODE_INFO AVFrameSideDataType
James Almer (12019-08-09): > Or just a pointer that points to the first byte after itself. The pointer takes places by itself. And it prevents the structure from being copied flatly, which IIRC is forbidden with side data. By the way, the lines of the commit message are too long. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures and AV_FRAME_DATA_ENCODE_INFO AVFrameSideDataType
On 8/9/2019 2:58 PM, Hendrik Leppkes wrote:
> On Fri, Aug 9, 2019 at 7:52 PM James Almer wrote:
>>
>> On 8/9/2019 2:38 PM, Juan De León wrote:
>>> AVEncodeInfoFrame data structure to store as AVFrameSideData of type
>>> AV_FRAME_DATA_ENCODE_INFO.
>>> The structure stores quantization index for each plane, DC/AC deltas for
>>> luma and chroma planes, and an array of AVEncodeInfoBlock struct denoting
>>> position, size, and delta quantizer for each block in the frame.
>>> Can be extended to support extraction of other block information.
>>>
>>> Signed-off-by: Juan De León
>>> ---
>>> fixed a typo in frame.h comment
>>> libavutil/Makefile | 2 +
>>> libavutil/encode_info.c | 67 +++
>>> libavutil/encode_info.h | 87 +
>>> libavutil/frame.c | 1 +
>>> libavutil/frame.h | 7
>>> 5 files changed, 164 insertions(+)
>>> create mode 100644 libavutil/encode_info.c
>>> create mode 100644 libavutil/encode_info.h
>>>
>>> diff --git a/libavutil/Makefile b/libavutil/Makefile
>>> index 57e6e3d7e8..37cfb099e9 100644
>>> --- a/libavutil/Makefile
>>> +++ b/libavutil/Makefile
>>> @@ -24,6 +24,7 @@ HEADERS = adler32.h
>>> \
>>>dict.h\
>>>display.h \
>>>downmix_info.h\
>>> + encode_info.h \
>>>encryption_info.h \
>>>error.h \
>>>eval.h\
>>> @@ -111,6 +112,7 @@ OBJS = adler32.o
>>> \
>>> dict.o \
>>> display.o\
>>> downmix_info.o \
>>> + encode_info.o\
>>> encryption_info.o\
>>> error.o \
>>> eval.o \
>>> diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
>>> new file mode 100644
>>> index 00..68c30af4d7
>>> --- /dev/null
>>> +++ b/libavutil/encode_info.c
>>> @@ -0,0 +1,67 @@
>>> +/*
>>> + * 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
>>> + */
>>> +
>>> +#include "libavutil/encode_info.h"
>>> +#include "libavutil/mem.h"
>>> +
>>> +static int init_encode_info_data(AVEncodeInfoFrame *ptr, int nb_blocks) {
>>> +ptr->nb_blocks = nb_blocks;
>>> +ptr->dc_q = ptr->ac_q = -1;
>>> +ptr->dc_chroma_q = ptr->ac_chroma_q = -1;
>>> +
>>> +for(int i=0;i>> +ptr->plane_q[i] = -1;
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
>>> +{
>>> +AVEncodeInfoFrame *ptr;
>>> +size_t size = sizeof(AVEncodeInfoFrame) +
>>> sizeof(AVEncodeInfoBlock)*nb_blocks;
>>> +
>>> +if (nb_blocks < 0 || size >= INT_MAX)
>>> +return NULL;
>>> +
>>> +ptr = av_mallocz(size);
>>> +if (!ptr)
>>> +return NULL;
>>> +
>>> +init_encode_info_data(ptr, nb_blocks);
>>> +
>>> +return ptr;
>>> +}
>>> +
>>> +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
>>> nb_blocks)
>>> +{
>>> +size_t size = sizeof(AVEncodeInfoFrame) +
>>> sizeof(AVEncodeInfoBlock)*nb_blocks;
>>> +
>>> +if (nb_blocks < 0 || size >= INT_MAX)
>>> +return NULL;
>>> +
>>> +AVFrameSideData *sd = av_frame_new_side_data(frame,
>>> + AV_FRAME_DATA_ENCODE_INFO,
>>> + size);
>>> +if (!sd)
>>> +return NULL;
>>> +
>>> +memset(sd->data, 0, size);
>>> +
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures and AV_FRAME_DATA_ENCODE_INFO AVFrameSideDataType
On Fri, Aug 9, 2019 at 7:52 PM James Almer wrote:
>
> On 8/9/2019 2:38 PM, Juan De León wrote:
> > AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> > AV_FRAME_DATA_ENCODE_INFO.
> > The structure stores quantization index for each plane, DC/AC deltas for
> > luma and chroma planes, and an array of AVEncodeInfoBlock struct denoting
> > position, size, and delta quantizer for each block in the frame.
> > Can be extended to support extraction of other block information.
> >
> > Signed-off-by: Juan De León
> > ---
> > fixed a typo in frame.h comment
> > libavutil/Makefile | 2 +
> > libavutil/encode_info.c | 67 +++
> > libavutil/encode_info.h | 87 +
> > libavutil/frame.c | 1 +
> > libavutil/frame.h | 7
> > 5 files changed, 164 insertions(+)
> > create mode 100644 libavutil/encode_info.c
> > create mode 100644 libavutil/encode_info.h
> >
> > diff --git a/libavutil/Makefile b/libavutil/Makefile
> > index 57e6e3d7e8..37cfb099e9 100644
> > --- a/libavutil/Makefile
> > +++ b/libavutil/Makefile
> > @@ -24,6 +24,7 @@ HEADERS = adler32.h
> > \
> >dict.h\
> >display.h \
> >downmix_info.h\
> > + encode_info.h \
> >encryption_info.h \
> >error.h \
> >eval.h\
> > @@ -111,6 +112,7 @@ OBJS = adler32.o
> > \
> > dict.o \
> > display.o\
> > downmix_info.o \
> > + encode_info.o\
> > encryption_info.o\
> > error.o \
> > eval.o \
> > diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> > new file mode 100644
> > index 00..68c30af4d7
> > --- /dev/null
> > +++ b/libavutil/encode_info.c
> > @@ -0,0 +1,67 @@
> > +/*
> > + * 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
> > + */
> > +
> > +#include "libavutil/encode_info.h"
> > +#include "libavutil/mem.h"
> > +
> > +static int init_encode_info_data(AVEncodeInfoFrame *ptr, int nb_blocks) {
> > +ptr->nb_blocks = nb_blocks;
> > +ptr->dc_q = ptr->ac_q = -1;
> > +ptr->dc_chroma_q = ptr->ac_chroma_q = -1;
> > +
> > +for(int i=0;i > +ptr->plane_q[i] = -1;
> > +
> > +return 0;
> > +}
> > +
> > +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
> > +{
> > +AVEncodeInfoFrame *ptr;
> > +size_t size = sizeof(AVEncodeInfoFrame) +
> > sizeof(AVEncodeInfoBlock)*nb_blocks;
> > +
> > +if (nb_blocks < 0 || size >= INT_MAX)
> > +return NULL;
> > +
> > +ptr = av_mallocz(size);
> > +if (!ptr)
> > +return NULL;
> > +
> > +init_encode_info_data(ptr, nb_blocks);
> > +
> > +return ptr;
> > +}
> > +
> > +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
> > nb_blocks)
> > +{
> > +size_t size = sizeof(AVEncodeInfoFrame) +
> > sizeof(AVEncodeInfoBlock)*nb_blocks;
> > +
> > +if (nb_blocks < 0 || size >= INT_MAX)
> > +return NULL;
> > +
> > +AVFrameSideData *sd = av_frame_new_side_data(frame,
> > + AV_FRAME_DATA_ENCODE_INFO,
> > + size);
> > +if (!sd)
> > +return NULL;
> > +
> > +memset(sd->data, 0, size);
> > +init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
> > +
> > +return
Re: [FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures and AV_FRAME_DATA_ENCODE_INFO AVFrameSideDataType
On 8/9/2019 2:38 PM, Juan De León wrote:
> AVEncodeInfoFrame data structure to store as AVFrameSideData of type
> AV_FRAME_DATA_ENCODE_INFO.
> The structure stores quantization index for each plane, DC/AC deltas for luma
> and chroma planes, and an array of AVEncodeInfoBlock struct denoting
> position, size, and delta quantizer for each block in the frame.
> Can be extended to support extraction of other block information.
>
> Signed-off-by: Juan De León
> ---
> fixed a typo in frame.h comment
> libavutil/Makefile | 2 +
> libavutil/encode_info.c | 67 +++
> libavutil/encode_info.h | 87 +
> libavutil/frame.c | 1 +
> libavutil/frame.h | 7
> 5 files changed, 164 insertions(+)
> create mode 100644 libavutil/encode_info.c
> create mode 100644 libavutil/encode_info.h
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 57e6e3d7e8..37cfb099e9 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -24,6 +24,7 @@ HEADERS = adler32.h
> \
>dict.h\
>display.h \
>downmix_info.h\
> + encode_info.h \
>encryption_info.h \
>error.h \
>eval.h\
> @@ -111,6 +112,7 @@ OBJS = adler32.o
> \
> dict.o \
> display.o\
> downmix_info.o \
> + encode_info.o\
> encryption_info.o\
> error.o \
> eval.o \
> diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
> new file mode 100644
> index 00..68c30af4d7
> --- /dev/null
> +++ b/libavutil/encode_info.c
> @@ -0,0 +1,67 @@
> +/*
> + * 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
> + */
> +
> +#include "libavutil/encode_info.h"
> +#include "libavutil/mem.h"
> +
> +static int init_encode_info_data(AVEncodeInfoFrame *ptr, int nb_blocks) {
> +ptr->nb_blocks = nb_blocks;
> +ptr->dc_q = ptr->ac_q = -1;
> +ptr->dc_chroma_q = ptr->ac_chroma_q = -1;
> +
> +for(int i=0;i +ptr->plane_q[i] = -1;
> +
> +return 0;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
> +{
> +AVEncodeInfoFrame *ptr;
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*nb_blocks;
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +ptr = av_mallocz(size);
> +if (!ptr)
> +return NULL;
> +
> +init_encode_info_data(ptr, nb_blocks);
> +
> +return ptr;
> +}
> +
> +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
> nb_blocks)
> +{
> +size_t size = sizeof(AVEncodeInfoFrame) +
> sizeof(AVEncodeInfoBlock)*nb_blocks;
> +
> +if (nb_blocks < 0 || size >= INT_MAX)
> +return NULL;
> +
> +AVFrameSideData *sd = av_frame_new_side_data(frame,
> + AV_FRAME_DATA_ENCODE_INFO,
> + size);
> +if (!sd)
> +return NULL;
> +
> +memset(sd->data, 0, size);
> +init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
> +
> +return (AVEncodeInfoFrame*)sd->data;
> +}
> diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
> new file mode 100644
> index 00..cbe8be2891
> --- /dev/null
> +++ b/libavutil/encode_info.h
> @@ -0,0 +1,87 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software;
[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures and AV_FRAME_DATA_ENCODE_INFO AVFrameSideDataType
AVEncodeInfoFrame data structure to store as AVFrameSideData of type
AV_FRAME_DATA_ENCODE_INFO.
The structure stores quantization index for each plane, DC/AC deltas for luma
and chroma planes, and an array of AVEncodeInfoBlock struct denoting position,
size, and delta quantizer for each block in the frame.
Can be extended to support extraction of other block information.
Signed-off-by: Juan De León
---
libavutil/Makefile | 2 +
libavutil/encode_info.c | 67 +++
libavutil/encode_info.h | 87 +
libavutil/frame.c | 1 +
libavutil/frame.h | 7
5 files changed, 164 insertions(+)
create mode 100644 libavutil/encode_info.c
create mode 100644 libavutil/encode_info.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 57e6e3d7e8..37cfb099e9 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h
\
dict.h\
display.h \
downmix_info.h\
+ encode_info.h \
encryption_info.h \
error.h \
eval.h\
@@ -111,6 +112,7 @@ OBJS = adler32.o
\
dict.o \
display.o\
downmix_info.o \
+ encode_info.o\
encryption_info.o\
error.o \
eval.o \
diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
new file mode 100644
index 00..68c30af4d7
--- /dev/null
+++ b/libavutil/encode_info.c
@@ -0,0 +1,67 @@
+/*
+ * 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
+ */
+
+#include "libavutil/encode_info.h"
+#include "libavutil/mem.h"
+
+static int init_encode_info_data(AVEncodeInfoFrame *ptr, int nb_blocks) {
+ptr->nb_blocks = nb_blocks;
+ptr->dc_q = ptr->ac_q = -1;
+ptr->dc_chroma_q = ptr->ac_chroma_q = -1;
+
+for(int i=0;iplane_q[i] = -1;
+
+return 0;
+}
+
+AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
+{
+AVEncodeInfoFrame *ptr;
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*nb_blocks;
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+ptr = av_mallocz(size);
+if (!ptr)
+return NULL;
+
+init_encode_info_data(ptr, nb_blocks);
+
+return ptr;
+}
+
+AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
nb_blocks)
+{
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*nb_blocks;
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+AVFrameSideData *sd = av_frame_new_side_data(frame,
+ AV_FRAME_DATA_ENCODE_INFO,
+ size);
+if (!sd)
+return NULL;
+
+memset(sd->data, 0, size);
+init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
+
+return (AVEncodeInfoFrame*)sd->data;
+}
diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
new file mode 100644
index 00..cbe8be2891
--- /dev/null
+++ b/libavutil/encode_info.h
@@ -0,0 +1,87 @@
+/*
+ * 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
[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures and AV_FRAME_DATA_ENCODE_INFO AVFrameSideDataType
AVEncodeInfoFrame data structure to store as AVFrameSideData of type
AV_FRAME_DATA_ENCODE_INFO.
The structure stores quantization index for each plane, DC/AC deltas for luma
and chroma planes, and an array of AVEncodeInfoBlock struct denoting position,
size, and delta quantizer for each block in the frame.
Can be extended to support extraction of other block information.
Signed-off-by: Juan De León
---
fixed a typo in frame.h comment
libavutil/Makefile | 2 +
libavutil/encode_info.c | 67 +++
libavutil/encode_info.h | 87 +
libavutil/frame.c | 1 +
libavutil/frame.h | 7
5 files changed, 164 insertions(+)
create mode 100644 libavutil/encode_info.c
create mode 100644 libavutil/encode_info.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 57e6e3d7e8..37cfb099e9 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -24,6 +24,7 @@ HEADERS = adler32.h
\
dict.h\
display.h \
downmix_info.h\
+ encode_info.h \
encryption_info.h \
error.h \
eval.h\
@@ -111,6 +112,7 @@ OBJS = adler32.o
\
dict.o \
display.o\
downmix_info.o \
+ encode_info.o\
encryption_info.o\
error.o \
eval.o \
diff --git a/libavutil/encode_info.c b/libavutil/encode_info.c
new file mode 100644
index 00..68c30af4d7
--- /dev/null
+++ b/libavutil/encode_info.c
@@ -0,0 +1,67 @@
+/*
+ * 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
+ */
+
+#include "libavutil/encode_info.h"
+#include "libavutil/mem.h"
+
+static int init_encode_info_data(AVEncodeInfoFrame *ptr, int nb_blocks) {
+ptr->nb_blocks = nb_blocks;
+ptr->dc_q = ptr->ac_q = -1;
+ptr->dc_chroma_q = ptr->ac_chroma_q = -1;
+
+for(int i=0;iplane_q[i] = -1;
+
+return 0;
+}
+
+AVEncodeInfoFrame *av_encode_info_alloc(int nb_blocks)
+{
+AVEncodeInfoFrame *ptr;
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*nb_blocks;
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+ptr = av_mallocz(size);
+if (!ptr)
+return NULL;
+
+init_encode_info_data(ptr, nb_blocks);
+
+return ptr;
+}
+
+AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame, int
nb_blocks)
+{
+size_t size = sizeof(AVEncodeInfoFrame) +
sizeof(AVEncodeInfoBlock)*nb_blocks;
+
+if (nb_blocks < 0 || size >= INT_MAX)
+return NULL;
+
+AVFrameSideData *sd = av_frame_new_side_data(frame,
+ AV_FRAME_DATA_ENCODE_INFO,
+ size);
+if (!sd)
+return NULL;
+
+memset(sd->data, 0, size);
+init_encode_info_data((AVEncodeInfoFrame*)sd->data, nb_blocks);
+
+return (AVEncodeInfoFrame*)sd->data;
+}
diff --git a/libavutil/encode_info.h b/libavutil/encode_info.h
new file mode 100644
index 00..cbe8be2891
--- /dev/null
+++ b/libavutil/encode_info.h
@@ -0,0 +1,87 @@
+/*
+ * 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
