[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures
Juan De León
juandl at google.com
Mon Aug 19 22:00:15 EEST 2019
On Sat, Aug 17, 2019 at 7:00 AM Nicolas George <george at nsup.org> 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 <george at nsup.org> 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 <juandl at google.com>
> > ---
> > 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 0000000000..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)
> > + 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 0000000000..354411b9e1
> > --- /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 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> > + */
> > +
> > +#ifndef AVUTIL_ENCODE_INFO_H
> > +#define AVUTIL_ENCODE_INFO_H
> > +
> > +#include "libavutil/avassert.h"
> > +#include "libavutil/frame.h"
> > +
> > +/**
> > + * Data structure for extracting block data, stored as an array in
> AVEncodeInfoFrame.
> > + */
> > +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 with respect to the frame.
> > + */
> > + int delta_q;
> > +} AVEncodeInfoBlock;
> > +
> > +/**
> > + * Frame encoding info, used as AVFrameSideData. Data in this structure
> concerns
> > + * the whole frame.
> > + * Additional entries may be added without bumping major before
> nb_blocks,
> > + * so using the accessor function av_encode_info_get_block() is
> recommended.
> > + */
> > +typedef struct AVEncodeInfoFrame {
> > + /**
> > + * Base plane quantizer for the frame, set to -1 when value is
> unsupported.
> > + */
> > + int plane_q[AV_NUM_DATA_POINTERS];
> > + /**
> > + * DC/AC quantizer index delta, set to -1 when value is value
> unsupported.
> > + */
> > + int ac_q, dc_q;
> > + /**
> > + * DC/AC chroma quantizer index delta, set to -1 when value is
> value unsupported.
> > + */
> > + int ac_chroma_q, dc_chroma_q;
> > + /**
> > + * Number of blocks in the array, may be 0.
> > + */
> > + unsigned int nb_blocks;
> > + /**
> > + * Offset in this structure at which blocks begin in bytes. May not
> match
> > + * offsetof(AVEncodeInfoFrame, blocks).
> > + */
> > + size_t blocks_offset;
> > + /*
> > + * Size of each block in bytes. May not match
> sizeof(AVEncodeInfoBlock).
> > + */
> > + size_t block_size;
> > +
> > + /*
> > + * Array of blocks, with a total size of block_size*nb_blocks, the
> [1]
> > + * is meant for compatibility with C++.
> > + */
> > + AVEncodeInfoBlock blocks[1];
> > +} AVEncodeInfoFrame;
> > +
> > +/*
> > + * Gets the block at the specified {@code idx}. Must be between 0 and
> nb_blocks.
> > + */
> > +static inline AVEncodeInfoBlock
> *av_encode_info_get_block(AVEncodeInfoFrame *info, unsigned int idx)
> > +{
> > + av_assert0(idx < info->nb_blocks);
> > +
> > + return (AVEncodeInfoBlock *)((uint8_t *)info + info->blocks_offset
> + idx*info->block_size);
> > +}
> > +
> > +/**
> > + * Allocates memory for AVEncodeInfoFrame plus an array of
> > + * {@code nb_blocks} AVEncodeInfoBlock and initializes the variables.
> > + * Can be freed with a normal av_free() call.
> > + */
> > +AVEncodeInfoFrame *av_encode_info_alloc(unsigned int nb_blocks);
> > +
> > +/**
> > + * Allocates memory for AVEncodeInfoFrame plus an array of
> > + * {@code nb_blocks} AVEncodeInfoBlock in the given AVFrame {@code
> frame}
> > + * as AVFrameSideData of type AV_FRAME_DATA_ENCODE_INFO
> > + * and initializes the variables.
> > + */
> > +AVEncodeInfoFrame *av_encode_info_create_side_data(AVFrame *frame,
> unsigned int nb_blocks);
> > +
> > +#endif /* AVUTIL_ENCODE_INFO_H */
> > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > index dcf1fc3d17..65c25e6cd7 100644
> > --- a/libavutil/frame.c
> > +++ b/libavutil/frame.c
> > @@ -842,6 +842,7 @@ const char *av_frame_side_data_name(enum
> AVFrameSideDataType type)
> > #endif
> > case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata
> SMPTE2094-40 (HDR10+)";
> > case AV_FRAME_DATA_REGIONS_OF_INTEREST: return "Regions Of
> Interest";
> > + case AV_FRAME_DATA_ENCODE_INFO: return
> "AVEncodeInfo";
> > }
> > return NULL;
> > }
> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > index 5d3231e7bb..ec112c5d15 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -179,6 +179,13 @@ enum AVFrameSideDataType {
> > * array element is implied by AVFrameSideData.size /
> AVRegionOfInterest.self_size.
> > */
> > AV_FRAME_DATA_REGIONS_OF_INTEREST,
> > + /**
> > + * Extract frame and block encode info from supported decoders. The
> data
> > + * stored is an AVEncodeInfoFrame type, which contains an array of
> > + * AVEncodeInfoBlock. Described in libavuitls/encode_info.h
> > + * Can be allocated in the frame directly with
> av_encode_info_create_side_data().
> > + */
> > + AV_FRAME_DATA_ENCODE_INFO,
> > };
> >
> > enum AVActiveFormatDescription {
> > --
> > 2.23.0.rc1.153.gdeed80330f-goog
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
More information about the ffmpeg-devel
mailing list