[FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos
Andrey Semashev
andrey.semashev at gmail.com
Mon Jul 29 22:03:06 EEST 2019
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 at 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 */
>
More information about the ffmpeg-devel
mailing list