On Sat, Aug 17, 2019 at 7:00 AM Nicolas George <geo...@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 <geo...@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 <jua...@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@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".