| Message ID | 20190729180908.218946-1-juandl@google.com |
|---|---|
| State | Superseded |
| Headers | show |
Just a few random comments. Disclaimer: I'm not a maintainer. On 7/29/19 9:09 PM, Juan De León wrote: > Changes to libavcodec, hope this addresses all your comments. > Cheers. > > Signed-off-by: Juan De León <juandl@google.com> > --- > libavutil/Makefile | 2 + > libavutil/frame.h | 6 ++ > libavutil/quantization_params.c | 41 ++++++++++++ > libavutil/quantization_params.h | 106 ++++++++++++++++++++++++++++++++ > 4 files changed, 155 insertions(+) > create mode 100644 libavutil/quantization_params.c > create mode 100644 libavutil/quantization_params.h > > diff --git a/libavutil/Makefile b/libavutil/Makefile > index 8a7a44e4b5..be5e5d831f 100644 > --- a/libavutil/Makefile > +++ b/libavutil/Makefile > @@ -60,6 +60,7 @@ HEADERS = adler32.h \ > pixdesc.h \ > pixelutils.h \ > pixfmt.h \ > + quantization_params.o \ .h? > random_seed.h \ > rc4.h \ > rational.h \ > @@ -140,6 +141,7 @@ OBJS = adler32.o \ > parseutils.o \ > pixdesc.o \ > pixelutils.o \ > + quantization_params.o \ > random_seed.o \ > rational.o \ > reverse.o \ > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 5d3231e7bb..d48ccf342f 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -179,6 +179,12 @@ enum AVFrameSideDataType { > * array element is implied by AVFrameSideData.size / AVRegionOfInterest.self_size. > */ > AV_FRAME_DATA_REGIONS_OF_INTEREST, > + /** > + * To extract quantization parameters from supported decoders. > + * The data stored is AVQuantizationParamsArray type, described in The data is stored as AVQuantizationParamsArray, described... > + * libavuitls/quantization_params.h libavutil > + */ > + AV_FRAME_DATA_QUANTIZATION_PARAMS, > }; > > enum AVActiveFormatDescription { > diff --git a/libavutil/quantization_params.c b/libavutil/quantization_params.c > new file mode 100644 > index 0000000000..28b08ebe19 > --- /dev/null > +++ b/libavutil/quantization_params.c > @@ -0,0 +1,41 @@ > +/* > + * 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/quantization_params.h" > + > +static const char* const QP_NAMES_H264[] = {"qp"}; > + > +static const char* const QP_NAMES_VP9[] = {"qydc", "qyac", "quvdc", "quvac", > + "qiydc", "qiyac", "qiuvdc", "qiuvac"}; > + > +static const char* const QP_NAMES_AV1[] = {"qydc", "qyac", "qudc", "quac", "qvdc", "qvac", > + "qiydc", "qiyac", "qiudc", "qiuac", "qivdc", "qivac"}; > + > +char* get_qp_str(enum AVCodecID codec_id, int index) > +{ > + switch (codec_id) { > + case AV_CODEC_ID_H264: > + return QP_NAMES_H264[index]; > + case AV_CODEC_ID_VP9: > + return QP_NAMES_VP9[index]; > + case AV_CODEC_ID_AV1: > + return QP_NAMES_AV1[index]; > + default: > + return NULL; > + } > +} > diff --git a/libavutil/quantization_params.h b/libavutil/quantization_params.h > new file mode 100644 > index 0000000000..7a3daeaae5 > --- /dev/null > +++ b/libavutil/quantization_params.h > @@ -0,0 +1,106 @@ > +/* > + * 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_QUANTIZATION_PARAMS_H > +#define AVUTIL_QUANTIZATION_PARAMS_H > + > +#include "libavcodec/avcodec.h" > + > +// Max size for the type array: QP_ARR_SIZE_AV1 = 12 > +#define MAX_SIZE 12 The macro name is too generic. Is the macro needed at all? > +/** > + * Data structure for extracting Quantization Parameters, codec independent > + */ > +typedef struct AVQuantizationParams { > + /** > + * x and y coordinates of the block in pixels > + */ > + int x, y; > + /** > + * width and height of the block in pixels > + */ > + int w, h; > + /** > + * qp array, indexed by type according to > + * the enum corresponding to the codec > + */ > + int type[MAX_SIZE]; > +} AVQuantizationParams; > + > +/** > + * For storing an array of AVQuantization parameters and its size > + * Used as AVFrameSideData > + */ > +typedef struct AVQuantizationParamsArray { > + /** > + * AVQuantizationParams block array > + */ > + AVQuantizationParams *qp_arr; > + /** > + * size of the array > + */ > + int nb_blocks; > + > + enum AVCodecID codec_id; > +} AVQuantizationParamsArray; > + > +/** > + * Get the string describing the qp type for the given codec > + */ > +char* get_qp_str(enum AVCodecID codec_id, int index); Function name is too generic. I think, all public functions have to be prefixed with "av_". > +/** > + * Enums for different codecs to store qp in the type array > + * Each enum must have an array of strings describing each field > + * declared in quantization_params.c > + */ > +enum QP_ARR_INDEXES_FOR_H264 { AV_QP_ARR_INDEXES_H264? Enum values also should have AV_ prefix and a better relation to the enum type name. Similarly for the other enums. > + QP_H264 = 0, // qp value > + QP_ARR_SIZE_H264 // used for allocating memory > +}; > + > +enum QP_ARR_INDEXES_FOR_VP9 { > + QP_YDC_VP9 = 0, > + QP_YAC_VP9, > + QP_UVDC_VP9, > + QP_UVAC_VP9, > + QP_INDEX_YDC_VP9, > + QP_INDEX_YAC_VP9, > + QP_INDEX_UVDC_VP9, > + QP_INDEX_UVAC_VP9, > + QP_ARR_SIZE_VP9 > +}; > + > +enum QP_ARR_INDEXES_FOR_AV1 { > + QP_YDC_AV1 = 0, > + QP_YAC_AV1, > + QP_UDC_AV1, > + QP_UAC_AV1, > + QP_VDC_AV1, > + QP_VAC_AV1, > + QP_INDEX_YDC_AV1, > + QP_INDEX_YAC_AV1, > + QP_INDEX_UDC_AV1, > + QP_INDEX_UAC_AV1, > + QP_INDEX_VDC_AV1, > + QP_INDEX_VAC_AV1, > + QP_ARR_SIZE_AV1 > +}; > + > +#endif /* AVUTIL_QUANTIZATION_PARAMS_H */ >
On 29/07/2019 19:09, Juan De León wrote: > Changes to libavcodec, hope this addresses all your comments. > Cheers. This doesn't belong in the commit message. What does belong here would be some commentary on why you want this feature. > Signed-off-by: Juan De León <juandl@google.com> > --- > libavutil/Makefile | 2 + > libavutil/frame.h | 6 ++ > libavutil/quantization_params.c | 41 ++++++++++++ > libavutil/quantization_params.h | 106 ++++++++++++++++++++++++++++++++ > 4 files changed, 155 insertions(+) > create mode 100644 libavutil/quantization_params.c > create mode 100644 libavutil/quantization_params.h > > diff --git a/libavutil/Makefile b/libavutil/Makefile > index 8a7a44e4b5..be5e5d831f 100644 > --- a/libavutil/Makefile > +++ b/libavutil/Makefile > @@ -60,6 +60,7 @@ HEADERS = adler32.h \ > pixdesc.h \ > pixelutils.h \ > pixfmt.h \ > + quantization_params.o \ Object file in the list of headers? Also broken spacing - tabs are not allowed in C files. > random_seed.h \ > rc4.h \ > rational.h \ > @@ -140,6 +141,7 @@ OBJS = adler32.o \ > parseutils.o \ > pixdesc.o \ > pixelutils.o \ > + quantization_params.o \ > random_seed.o \ > rational.o \ > reverse.o \ > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 5d3231e7bb..d48ccf342f 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -179,6 +179,12 @@ enum AVFrameSideDataType { > * array element is implied by AVFrameSideData.size / AVRegionOfInterest.self_size. > */ > AV_FRAME_DATA_REGIONS_OF_INTEREST, > + /** > + * To extract quantization parameters from supported decoders. > + * The data stored is AVQuantizationParamsArray type, described in > + * libavuitls/quantization_params.h > + */ > + AV_FRAME_DATA_QUANTIZATION_PARAMS, > }; > > enum AVActiveFormatDescription { > diff --git a/libavutil/quantization_params.c b/libavutil/quantization_params.c > new file mode 100644 > index 0000000000..28b08ebe19 > --- /dev/null > +++ b/libavutil/quantization_params.c > @@ -0,0 +1,41 @@ > +/* > + * 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/quantization_params.h" > + > +static const char* const QP_NAMES_H264[] = {"qp"}; > + > +static const char* const QP_NAMES_VP9[] = {"qydc", "qyac", "quvdc", "quvac", > + "qiydc", "qiyac", "qiuvdc", "qiuvac"}; > + > +static const char* const QP_NAMES_AV1[] = {"qydc", "qyac", "qudc", "quac", "qvdc", "qvac", > + "qiydc", "qiyac", "qiudc", "qiuac", "qivdc", "qivac"}; > + > +char* get_qp_str(enum AVCodecID codec_id, int index) The values this function is returning are pointers to const char. > +{ > + switch (codec_id) { > + case AV_CODEC_ID_H264: > + return QP_NAMES_H264[index]; > + case AV_CODEC_ID_VP9: > + return QP_NAMES_VP9[index]; > + case AV_CODEC_ID_AV1: > + return QP_NAMES_AV1[index]; > + default: > + return NULL; > + } > +} > diff --git a/libavutil/quantization_params.h b/libavutil/quantization_params.h > new file mode 100644 > index 0000000000..7a3daeaae5 > --- /dev/null > +++ b/libavutil/quantization_params.h > @@ -0,0 +1,106 @@ > +/* > + * 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_QUANTIZATION_PARAMS_H > +#define AVUTIL_QUANTIZATION_PARAMS_H > + > +#include "libavcodec/avcodec.h" This is not allowed - libavutil cannot depend on libavcodec. > + > +// Max size for the type array: QP_ARR_SIZE_AV1 = 12 > +#define MAX_SIZE 12 Everything in header files must be namespaced. (This name will clash everywhere - indeed, there are multiple instances of this name in ffmpeg already.) > + > +/** > + * Data structure for extracting Quantization Parameters, codec independent > + */ > +typedef struct AVQuantizationParams { > + /** > + * x and y coordinates of the block in pixels > + */ > + int x, y; How do these values interact with cropping? > + /** > + * width and height of the block in pixels > + */ > + int w, h; > + /** > + * qp array, indexed by type according to > + * the enum corresponding to the codec > + */ > + int type[MAX_SIZE]; This name is confusing; it should probably at least suggest that it contains quantiser values. > +} AVQuantizationParams; > + > +/** > + * For storing an array of AVQuantization parameters and its size > + * Used as AVFrameSideData > + */ > +typedef struct AVQuantizationParamsArray { > + /** > + * AVQuantizationParams block array > + */ > + AVQuantizationParams *qp_arr; Side-data is reference counted, so how is this pointer managed? More genrally, it would probably help to explain exactly how this is allocated and who will be responsible for freeing it. > + /** > + * size of the array > + */ > + int nb_blocks; > + > + enum AVCodecID codec_id; > +} AVQuantizationParamsArray; > + > +/** > + * Get the string describing the qp type for the given codec > + */ > +char* get_qp_str(enum AVCodecID codec_id, int index); Requires namespacing. The name feels very unhelpful (characters are not a scarce resource when defining external API). It should also have a note on failure conditions - as written above, it returns NULL for invalid codec_id and invoked undefined behaviour for invalid index. > + > +/** > + * Enums for different codecs to store qp in the type array > + * Each enum must have an array of strings describing each field > + * declared in quantization_params.c > + */ > +enum QP_ARR_INDEXES_FOR_H264 { > + QP_H264 = 0, // qp value What value specifically does this refer to? QP_Y or QP'_Y for the given block? > + QP_ARR_SIZE_H264 // used for allocating memory Given that you have chroma quantiser values for the other codecs, why not for H.264? > +}; > + > +enum QP_ARR_INDEXES_FOR_VP9 { > + QP_YDC_VP9 = 0, > + QP_YAC_VP9, > + QP_UVDC_VP9, > + QP_UVAC_VP9, > + QP_INDEX_YDC_VP9, > + QP_INDEX_YAC_VP9, > + QP_INDEX_UVDC_VP9, > + QP_INDEX_UVAC_VP9, Can you explain exactly what each of these are with reference to the standard? (Something like the return value of get_[ad]c_quant(N) for the block?) It might help not to call them QPs, since that isn't a term used in VP9. > + QP_ARR_SIZE_VP9 > +}; > + > +enum QP_ARR_INDEXES_FOR_AV1 { > + QP_YDC_AV1 = 0, > + QP_YAC_AV1, > + QP_UDC_AV1, > + QP_UAC_AV1, > + QP_VDC_AV1, > + QP_VAC_AV1, > + QP_INDEX_YDC_AV1, > + QP_INDEX_YAC_AV1, > + QP_INDEX_UDC_AV1, > + QP_INDEX_UAC_AV1, > + QP_INDEX_VDC_AV1, > + QP_INDEX_VAC_AV1, Precise explanation of all of these needed too. > + QP_ARR_SIZE_AV1 > +}; Everything in these enums also requires namespacing. > + > +#endif /* AVUTIL_QUANTIZATION_PARAMS_H */ > What is the proposed consumer of this? It might help if you provided that as well (a filter?). - Mark
On Mon, Jul 29, 2019 at 12:48 PM Mark Thompson <sw@jkqxz.net> wrote: > This doesn't belong in the commit message. > > What does belong here would be some commentary on why you want this > feature. > Here is the, somewhat outdated, design document, this should explain it. https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing <https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit#> In short the purpose is to implement an API to extract QP and calculate min max and average. > + int x, y; > > How do these values interact with cropping? I'm not sure I understand, could you elaborate? > + AVQuantizationParams *qp_arr; > > Side-data is reference counted, so how is this pointer managed? More > genrally, it would probably help to explain exactly how this is allocated > and who will be responsible for freeing it. > The idea is to allocate the memory, for AVQuantizationParamsArray and the necessary number of AVQuantizationParams, in a single buffer that can be freed when the side data is freed. > > +enum QP_ARR_INDEXES_FOR_H264 { > > + QP_H264 = 0, // qp value > > What value specifically does this refer to? QP_Y or QP'_Y for the given > block? > It refers to the final QP of the block, not the delta. Also Added a field for chroma QP, thanks! > What is the proposed consumer of this? It might help if you provided that > as well (a filter?). > The filter is incomplete but I can submit what I have so far if needed to give more context. The document explains it better. https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing
On 7/29/2019 11:19 PM, Juan De León wrote: > On Mon, Jul 29, 2019 at 12:48 PM Mark Thompson <sw@jkqxz.net> wrote: > >> This doesn't belong in the commit message. >> >> What does belong here would be some commentary on why you want this >> feature. >> > Here is the, somewhat outdated, design document, this should explain it. > https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing > <https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit#> > In short the purpose is to implement an API to extract QP and calculate min > max and average. > >> + int x, y; >> >> How do these values interact with cropping? > > I'm not sure I understand, could you elaborate? > >> + AVQuantizationParams *qp_arr; >> >> Side-data is reference counted, so how is this pointer managed? More >> genrally, it would probably help to explain exactly how this is allocated >> and who will be responsible for freeing it. >> > The idea is to allocate the memory, for AVQuantizationParamsArray and the > necessary number of AVQuantizationParams, in a single buffer that can be > freed when the side data is freed. Side data, or more specifically, any AVBufferRef, must be a flat array. You can't have pointers to some other allocated buffer within them since you can't really control their lifetime.
On Mon, Jul 29, 2019 at 7:59 PM James Almer <jamrial@gmail.com> wrote: > Side data, or more specifically, any AVBufferRef, must be a flat array. > You can't have pointers to some other allocated buffer within them since > you can't really control their lifetime. > Here's a snippet of how I'm doing it right now: // mb_xy = number of blocks in the frame size_t buf_size = sizeof(AVQuantizationParamsArray) + mb_xy * sizeof(AVQuantizationParams); AVBufferRef *buffer = av_buffer_alloc(buf_size); AVQuantizationParamsArray *params = (AVQuantizationParamsArray *)buffer->data; // offset memory for qp_block_array in same buffer params->qp_block_array = (AVQuantizationParams*) (params + 1); AVFrameSideData *sd; sd = av_frame_new_side_data_from_buf(dst, AV_FRAME_DATA_QUANTIZATION_PARAMS, buffer); The idea is to keep everything in the same buffer so that it can all be freed when AVFrameSideData is freed. Let me know if this doesn't look right to you.
On Tue, Jul 30, 2019 at 10:49:00AM -0700, Juan De León wrote: > On Mon, Jul 29, 2019 at 7:59 PM James Almer <jamrial@gmail.com> wrote: > > > Side data, or more specifically, any AVBufferRef, must be a flat array. > > You can't have pointers to some other allocated buffer within them since > > you can't really control their lifetime. > > > Here's a snippet of how I'm doing it right now: > // mb_xy = number of blocks in the frame > size_t buf_size = sizeof(AVQuantizationParamsArray) + mb_xy * > sizeof(AVQuantizationParams); > AVBufferRef *buffer = av_buffer_alloc(buf_size); > AVQuantizationParamsArray *params = (AVQuantizationParamsArray > *)buffer->data; > > // offset memory for qp_block_array in same buffer > params->qp_block_array = (AVQuantizationParams*) (params + 1); iam not sure this is safe, it would be better if there are no pointers. This would break if the AVQuantizationParamsArray is moved in memory or cloned and the orginal freed or cloned and the clone changed with the expextiaiion that the original stays unchanged thx [...]
diff --git a/libavutil/Makefile b/libavutil/Makefile index 8a7a44e4b5..be5e5d831f 100644 --- a/libavutil/Makefile +++ b/libavutil/Makefile @@ -60,6 +60,7 @@ HEADERS = adler32.h \ pixdesc.h \ pixelutils.h \ pixfmt.h \ + quantization_params.o \ random_seed.h \ rc4.h \ rational.h \ @@ -140,6 +141,7 @@ OBJS = adler32.o \ parseutils.o \ pixdesc.o \ pixelutils.o \ + quantization_params.o \ random_seed.o \ rational.o \ reverse.o \ diff --git a/libavutil/frame.h b/libavutil/frame.h index 5d3231e7bb..d48ccf342f 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -179,6 +179,12 @@ enum AVFrameSideDataType { * array element is implied by AVFrameSideData.size / AVRegionOfInterest.self_size. */ AV_FRAME_DATA_REGIONS_OF_INTEREST, + /** + * To extract quantization parameters from supported decoders. + * The data stored is AVQuantizationParamsArray type, described in + * libavuitls/quantization_params.h + */ + AV_FRAME_DATA_QUANTIZATION_PARAMS, }; enum AVActiveFormatDescription { diff --git a/libavutil/quantization_params.c b/libavutil/quantization_params.c new file mode 100644 index 0000000000..28b08ebe19 --- /dev/null +++ b/libavutil/quantization_params.c @@ -0,0 +1,41 @@ +/* + * 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/quantization_params.h" + +static const char* const QP_NAMES_H264[] = {"qp"}; + +static const char* const QP_NAMES_VP9[] = {"qydc", "qyac", "quvdc", "quvac", + "qiydc", "qiyac", "qiuvdc", "qiuvac"}; + +static const char* const QP_NAMES_AV1[] = {"qydc", "qyac", "qudc", "quac", "qvdc", "qvac", + "qiydc", "qiyac", "qiudc", "qiuac", "qivdc", "qivac"}; + +char* get_qp_str(enum AVCodecID codec_id, int index) +{ + switch (codec_id) { + case AV_CODEC_ID_H264: + return QP_NAMES_H264[index]; + case AV_CODEC_ID_VP9: + return QP_NAMES_VP9[index]; + case AV_CODEC_ID_AV1: + return QP_NAMES_AV1[index]; + default: + return NULL; + } +} diff --git a/libavutil/quantization_params.h b/libavutil/quantization_params.h new file mode 100644 index 0000000000..7a3daeaae5 --- /dev/null +++ b/libavutil/quantization_params.h @@ -0,0 +1,106 @@ +/* + * 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_QUANTIZATION_PARAMS_H +#define AVUTIL_QUANTIZATION_PARAMS_H + +#include "libavcodec/avcodec.h" + +// Max size for the type array: QP_ARR_SIZE_AV1 = 12 +#define MAX_SIZE 12 + +/** + * Data structure for extracting Quantization Parameters, codec independent + */ +typedef struct AVQuantizationParams { + /** + * x and y coordinates of the block in pixels + */ + int x, y; + /** + * width and height of the block in pixels + */ + int w, h; + /** + * qp array, indexed by type according to + * the enum corresponding to the codec + */ + int type[MAX_SIZE]; +} AVQuantizationParams; + +/** + * For storing an array of AVQuantization parameters and its size + * Used as AVFrameSideData + */ +typedef struct AVQuantizationParamsArray { + /** + * AVQuantizationParams block array + */ + AVQuantizationParams *qp_arr; + /** + * size of the array + */ + int nb_blocks; + + enum AVCodecID codec_id; +} AVQuantizationParamsArray; + +/** + * Get the string describing the qp type for the given codec + */ +char* get_qp_str(enum AVCodecID codec_id, int index); + +/** + * Enums for different codecs to store qp in the type array + * Each enum must have an array of strings describing each field + * declared in quantization_params.c + */ +enum QP_ARR_INDEXES_FOR_H264 { + QP_H264 = 0, // qp value + QP_ARR_SIZE_H264 // used for allocating memory +}; + +enum QP_ARR_INDEXES_FOR_VP9 { + QP_YDC_VP9 = 0, + QP_YAC_VP9, + QP_UVDC_VP9, + QP_UVAC_VP9, + QP_INDEX_YDC_VP9, + QP_INDEX_YAC_VP9, + QP_INDEX_UVDC_VP9, + QP_INDEX_UVAC_VP9, + QP_ARR_SIZE_VP9 +}; + +enum QP_ARR_INDEXES_FOR_AV1 { + QP_YDC_AV1 = 0, + QP_YAC_AV1, + QP_UDC_AV1, + QP_UAC_AV1, + QP_VDC_AV1, + QP_VAC_AV1, + QP_INDEX_YDC_AV1, + QP_INDEX_YAC_AV1, + QP_INDEX_UDC_AV1, + QP_INDEX_UAC_AV1, + QP_INDEX_VDC_AV1, + QP_INDEX_VAC_AV1, + QP_ARR_SIZE_AV1 +}; + +#endif /* AVUTIL_QUANTIZATION_PARAMS_H */
Changes to libavcodec, hope this addresses all your comments. Cheers. Signed-off-by: Juan De León <juandl@google.com> --- libavutil/Makefile | 2 + libavutil/frame.h | 6 ++ libavutil/quantization_params.c | 41 ++++++++++++ libavutil/quantization_params.h | 106 ++++++++++++++++++++++++++++++++ 4 files changed, 155 insertions(+) create mode 100644 libavutil/quantization_params.c create mode 100644 libavutil/quantization_params.h