Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

2019-08-07 Thread Lynne 
Aug 7, 2019, 11:46 PM by juandl-at-google@ffmpeg.org:

> On Tue, Aug 6, 2019 at 5:07 PM Mark Thompson  wrote:
>
>> This should be in libavcodec, not libavutil - it relates directly to
>> codecs.  (Indeed, you've ended up having to define a special non-libavcodec
>> enum of codecs below to make it work in libavutil at all.)
>>
> If this belongs in avcodec I can move it there, but I don't see a similar
> data structure in that library.
> I believe declaring different IDs for supported codecs here is a better
> approach.
>
>
>> > +enum AVQPArrIndexesH264 {  // varaible names in spec document
>>
>> I don't think giving these enums a tag has any value?
>>
> They are not used in the code, but keeping them makes the purpose of each
> enum clearer.
>
>
>> > +AV_QP_ARR_SIZE_H264
>> > +};
>> > +
>> > +enum AVQPArrIndexesVP9 {   // variable names in spec document
>> > +AV_QP_YAC_VP9 = 0, // get_dc_quant[][base_q_idx]
>>
>> This isn't right - get_dc_quant() is a function of one variable, not a
>> two-dimensional array (confused with dc_qlookup[][] somehow?).
>>
> Thank you, I think I meant: ac_q(get_qindex()).
>
> Why are you including higher-level frame values for VP9 and AV1, but not
>
>> including similar ones for H.264?
>>
> Again, I meant get_qindex(), it is supposed to represent the quant index
> for the specified segment, not frame quant index.
>
> What is the motivation for AV1 returning the exponential coefficient
>
>> scaling values (range 4..29247) rather than the linear parameter (range
>> 0..255) as you do for H.264?
>>
>
> Exposing both the values was a requirement by my team.
>
>> +#define AV_QP_ARR_MAX_SIZE AV_QP_ARR_SIZE_AV1
>>
>> Fixing this for all time for a particular codec which happens to need the
>> most space when it is defined doesn't seem like a good idea.  E.g. you
>> can't support JPEG with only this number (it would need all entries in up
>> to four tables).
>>
>
> It might be better if the structure size wasn't fixed forever by the first
>
>> version of the API/ABI.  Perhaps an approach something like that used for
>> AVRegionOfInterest would work?
>>
> Each instance of AVQuantizationParams has an array of qp values/indexes
> (qp_type[]) for which I need a constant to allocate memory.
> The approach AVRegionOfInterest uses does not solve that problem.
>
>> +/**
>> > + * x and y coordinates of the block in pixels
>> > + */
>> > +int x, y;
>>
>> Don't call these x/y coordinates because it not clear exactly what that
>> means (what is the scale, where is the origin, which direction is positive,
>> where in the block is being referred to, etc.).
>>
>
> Instead follow the same convention as other structures in FFmpeg and define
>
>> them as the distance in pixels from the left or the top edge of the picture
>> to the top-left corner of the block.
>>
> That's exactly their purpose, the distance in pixels from the top-left
> corner of the frame, to the top-left corner of the block. I will make the
> description clearer, thank you.
>
>
>> On 30/07/2019 03:19, Juan De León wrote:
>> > On Mon, Jul 29, 2019 at 12:48 PM Mark Thompson  wrote:
>> >>
>> >> How do these values interact with cropping?
>> >
>> > I'm not sure I understand, could you elaborate?
>>
>> For codecs which include cropping such as H.26[45], the decoder may
>> directly apply cropping from the stream (controlled by
>> AVCodecContext.apply_cropping), possibly modified by alignment (with
>> AV_CODEC_FLAG_UNALIGNED), and then sets the AVFrame cropping fields to
>> reflect the remainder.
>>
>
> For example, in H.264 try setting the
>
>> frame_crop_left_offset/frame_crop_top_offset fields in a stream to large
>> values (h264_metadata can do this for an existing stream).  What do your
>> x/y values then refer to in the result?  They could be negative to indicate
>> macroblocks which are off the edges of the cropped picture, or they might
>> be relative to the uncropped picture in which case you would need
>> additional information to reconstruct which blocks they refer to in the
>> frame you actually have.
>>
> The coordinates of the blocks should correspond to the coded picture,
> quantization is still applied to cropped MBs outside of the frame so that
> should be considered for the logging and avg calculation, similar to an
> analyzer.
>
>
>> > +/**
>> > + * Stores an id corresponding to one of the supported codecs
>> > + */
>> > +enum AVExtractQPSupportedCodecs codec_id;
>>
>> enum AVCodecID, with this in libavcodec.
>>
> Like Michael said, this could cause conflict when extracting QP. It might
> be better to leave it as a separate ID.
>

No one will start writing an mpeg2 encoder now, so I disagree. A codec ID is 
better.



>> +/**
>> > + * Get the string describing the qp type for the given codec
>> > + */
>> > +const char* av_get_qp_type_string(enum AVExtractQPSupportedCodecs
>> codec_id, int index);
>>
>> I'm not sure there is a good reason to embed this in the public API - what

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

2019-08-07 Thread Juan De León 
On Tue, Aug 6, 2019 at 5:07 PM Mark Thompson  wrote:

> This should be in libavcodec, not libavutil - it relates directly to
> codecs.  (Indeed, you've ended up having to define a special non-libavcodec
> enum of codecs below to make it work in libavutil at all.)
>
If this belongs in avcodec I can move it there, but I don't see a similar
data structure in that library.
I believe declaring different IDs for supported codecs here is a better
approach.


> > +enum AVQPArrIndexesH264 {  // varaible names in spec document
>
> I don't think giving these enums a tag has any value?
>
They are not used in the code, but keeping them makes the purpose of each
enum clearer.


> > +AV_QP_ARR_SIZE_H264
> > +};
> > +
> > +enum AVQPArrIndexesVP9 {   // variable names in spec document
> > +AV_QP_YAC_VP9 = 0, // get_dc_quant[][base_q_idx]
>
> This isn't right - get_dc_quant() is a function of one variable, not a
> two-dimensional array (confused with dc_qlookup[][] somehow?).
>
Thank you, I think I meant: ac_q(get_qindex()).

Why are you including higher-level frame values for VP9 and AV1, but not
> including similar ones for H.264?
>
Again, I meant get_qindex(), it is supposed to represent the quant index
for the specified segment, not frame quant index.

What is the motivation for AV1 returning the exponential coefficient
> scaling values (range 4..29247) rather than the linear parameter (range
> 0..255) as you do for H.264?

Exposing both the values was a requirement by my team.

> +#define AV_QP_ARR_MAX_SIZE AV_QP_ARR_SIZE_AV1
>
> Fixing this for all time for a particular codec which happens to need the
> most space when it is defined doesn't seem like a good idea.  E.g. you
> can't support JPEG with only this number (it would need all entries in up
> to four tables).

It might be better if the structure size wasn't fixed forever by the first
> version of the API/ABI.  Perhaps an approach something like that used for
> AVRegionOfInterest would work?
>
Each instance of AVQuantizationParams has an array of qp values/indexes
(qp_type[]) for which I need a constant to allocate memory.
The approach AVRegionOfInterest uses does not solve that problem.

> +/**
> > + * x and y coordinates of the block in pixels
> > + */
> > +int x, y;
>
> Don't call these x/y coordinates because it not clear exactly what that
> means (what is the scale, where is the origin, which direction is positive,
> where in the block is being referred to, etc.).

Instead follow the same convention as other structures in FFmpeg and define
> them as the distance in pixels from the left or the top edge of the picture
> to the top-left corner of the block.
>
That's exactly their purpose, the distance in pixels from the top-left
corner of the frame, to the top-left corner of the block. I will make the
description clearer, thank you.


> On 30/07/2019 03:19, Juan De León wrote:
> > On Mon, Jul 29, 2019 at 12:48 PM Mark Thompson  wrote:
> >>
> >> How do these values interact with cropping?
> >
> > I'm not sure I understand, could you elaborate?
>
> For codecs which include cropping such as H.26[45], the decoder may
> directly apply cropping from the stream (controlled by
> AVCodecContext.apply_cropping), possibly modified by alignment (with
> AV_CODEC_FLAG_UNALIGNED), and then sets the AVFrame cropping fields to
> reflect the remainder.

For example, in H.264 try setting the
> frame_crop_left_offset/frame_crop_top_offset fields in a stream to large
> values (h264_metadata can do this for an existing stream).  What do your
> x/y values then refer to in the result?  They could be negative to indicate
> macroblocks which are off the edges of the cropped picture, or they might
> be relative to the uncropped picture in which case you would need
> additional information to reconstruct which blocks they refer to in the
> frame you actually have.
>
The coordinates of the blocks should correspond to the coded picture,
quantization is still applied to cropped MBs outside of the frame so that
should be considered for the logging and avg calculation, similar to an
analyzer.


> > +/**
> > + * Stores an id corresponding to one of the supported codecs
> > + */
> > +enum AVExtractQPSupportedCodecs codec_id;
>
> enum AVCodecID, with this in libavcodec.
>
Like Michael said, this could cause conflict when extracting QP. It might
be better to leave it as a separate ID.

> +/**
> > + * Get the string describing the qp type for the given codec
> > + */
> > +const char* av_get_qp_type_string(enum AVExtractQPSupportedCodecs
> codec_id, int index);
>
> I'm not sure there is a good reason to embed this in the public API - what
> user is ever going to call this function?  Anyone using the enum values
> must already know exactly what each of them mean to do anything with them
> at all, so if they need string names they'll already have clearer ones than
> the cryptic short names you provide here.

I think it would probably be better to just

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

2019-08-07 Thread Michael Niedermayer 
On Wed, Aug 07, 2019 at 12:59:33AM +0100, Mark Thompson wrote:
> On 05/08/2019 20:20, Juan De León wrote:
[...]
> 
> > +/**
> > + * Stores an id corresponding to one of the supported codecs
> > + */
> > +enum AVExtractQPSupportedCodecs codec_id;
> 
> enum AVCodecID, with this in libavcodec.

This may have interresting corner cases
like mpeg4 which can use h263 style or mpeg2 style quantization depending on a 
flag

so for these we may want the stored type to differ from the actual
decoder codec_id

and then the question of course arrises if a codec uses the same type
of quantization as another, should it use the same id as that other
or its own id. I think both have advantages and disadvantages

Thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein


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] Extract QP from h264 encoded videos

2019-08-06 Thread Mark Thompson 
On 05/08/2019 20:20, Juan De León wrote:
> AVQuantizationParams data structure for extracting qp and storing as 
> AV_FRAME_DATA_QUANTIZATION_PARAMS AVFrameSideDataType
> design doc: 
> https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing
> 
> Signed-off-by: Juan De León 
> ---
>  libavutil/Makefile  |   2 +
>  libavutil/frame.h   |   6 ++
>  libavutil/quantization_params.c |  83 
>  libavutil/quantization_params.h | 110 
>  4 files changed, 201 insertions(+)
>  create mode 100644 libavutil/quantization_params.c
>  create mode 100644 libavutil/quantization_params.h

This should be in libavcodec, not libavutil - it relates directly to codecs.  
(Indeed, you've ended up having to define a special non-libavcodec enum of 
codecs below to make it work in libavutil at all.)

> diff --git a/libavutil/quantization_params.h b/libavutil/quantization_params.h
> new file mode 100644
> index 00..1c1b474dca
> --- /dev/null
> +++ b/libavutil/quantization_params.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_QUANTIZATION_PARAMS_H
> +#define AVUTIL_QUANTIZATION_PARAMS_H
> +
> +/**
> + * Supported decoders for extraction and filter
> + */
> +enum AVExtractQPSupportedCodecs {
> +AV_EXTRACT_QP_CODEC_ID_H264 = 0,
> +AV_EXTRACT_QP_CODEC_ID_VP9,
> +AV_EXTRACT_QP_CODEC_ID_AV1,
> +};
> +
> +/**
> + * Enums for different codecs to store qp in the type array
> + * Each enum must have an array of strings describing each field
> + * declared in libavutil/quantization_params.c
> + */
> +
> +enum AVQPArrIndexesH264 {  // varaible names in spec document

I don't think giving these enums a tag has any value?

> +AV_QP_Y_H264 = 0,  // QPy
> +AV_QP_U_H264,  // QPcb

There is no variable in the standard with this name, which will confuse 
attempts to search for its meaning.  I think you mean QPc for the Cb component, 
as described in section 8.5.8.

> +AV_QP_V_H264,  // QPcr

Likewise here.

> +AV_QP_ARR_SIZE_H264
> +};
> +
> +enum AVQPArrIndexesVP9 {   // variable names in spec document
> +AV_QP_YAC_VP9 = 0, // get_dc_quant[][base_q_idx]

This isn't right - get_dc_quant() is a function of one variable, not a 
two-dimensional array (confused with dc_qlookup[][] somehow?).

> +AV_QP_YDC_VP9, // get_dc_quant[][base_q_idx+delta_q_y_dc]
> +AV_QP_UVDC_VP9,// get_dc_quant[][base_q_idx+delta_q_uv_dc]
> +AV_QP_UVAC_VP9,// get_ac_quant[][base_q_idx+delta_q_uv_ac]
> +AV_QP_INDEX_YAC_VP9,   // base_q_idx
> +AV_QP_INDEX_YDC_VP9,   // base_q_idx+delta_q_y_dc
> +AV_QP_INDEX_UVDC_VP9,  // base_q_idx+delta_q_uv_dc
> +AV_QP_INDEX_UVAC_VP9,  // base_q_idx+delta_q_uv_ac

Why are you including higher-level frame values for VP9 and AV1, but not 
including similar ones for H.264?

> +AV_QP_ARR_SIZE_VP9
> +};
> +
> +enum AVQPArrIndexesAV1 {  // variable names in spec document
> +AV_QP_YAC_AV1 = 0,// dc_q(base_q_idx)

What is the motivation for AV1 returning the exponential coefficient scaling 
values (range 4..29247) rather than the linear parameter (range 0..255) as you 
do for H.264?

> +AV_QP_YDC_AV1,// dc_q(base_q_idx+DeltaQYDc)
> +AV_QP_UDC_AV1,// dc_q(base_q_idx+DeltaQUDc)
> +AV_QP_UAC_AV1,// dc_q(base_q_idx+DeltaQUAc)
> +AV_QP_VDC_AV1,// dc_q(base_q_idx+DeltaQVDc)
> +AV_QP_VAC_AV1,// dc_q(base_q_idx+DeltaQVAc)
> +AV_QP_INDEX_YAC_AV1,  // base_q_idx
> +AV_QP_INDEX_YDC_AV1,  // base_q_idx+DeltaQYDc
> +AV_QP_INDEX_UDC_AV1,  // base_q_idx+DeltaQUDc
> +AV_QP_INDEX_UAC_AV1,  // base_q_idx+DeltaQUAc
> +AV_QP_INDEX_VDC_AV1,  // base_q_idx+DeltaQVDc
> +AV_QP_INDEX_VAC_AV1,  // base_q_idx+DeltaQVAc
> +AV_QP_ARR_SIZE_AV1
> +};

The term "QP" is never used in VP9 or AV1.  Maybe these could have a less 
H.26[45]-oriented name?

> +
> +/**
> + * Update AV_QP_ARR_MAX_SIZE when a new enum is defined that
> + * exceeds the current max size.
> + */
> +
> +#define AV_QP_ARR_MAX_SIZE AV_QP_ARR_SIZE_AV1

Fixing this for all time for a particular

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

2019-08-06 Thread Juan De León 
On Mon, Aug 5, 2019 at 1:07 PM Michael Niedermayer 
wrote

> a macro would still require a major version bump as its a ABI change, if
> it changes.
> To fix this would probably require a deeper change, we could also
> of course just leave it and accept that as the maximum till the next
> ABI major bump
>

I added the macro to at least make it clearer, I don't see any other
cleaner solution at the moment.
Please review my latest patch and if there are any more concerns please let
me know.
___
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] Extract QP from h264 encoded videos

2019-08-05 Thread Michael Niedermayer 
On Mon, Aug 05, 2019 at 12:16:08PM -0700, Juan De León wrote:
> On Sat, Aug 3, 2019 at 12:15 PM Michael Niedermayer 
> wrote:
> 
> > > +const char* av_get_qp_type_string(enum AVExtractQPSupportedCodecs
> > codec_id, int index)
> > > +{
> > > +switch (codec_id) {
> > > +case AV_EXTRACT_QP_CODEC_ID_H264:
> > > +return index < AV_QP_ARR_SIZE_H264 ? QP_NAMES_H264[index]
> > :NULL;
> > > +case AV_EXTRACT_QP_CODEC_ID_VP9:
> > > +return index < AV_QP_ARR_SIZE_VP9  ? QP_NAMES_VP9[index]
> > :NULL;
> > > +case AV_EXTRACT_QP_CODEC_ID_AV1:
> > > +return index < AV_QP_ARR_SIZE_AV1  ? QP_NAMES_AV1[index]
> > :NULL;
> > > +default:
> > > +return NULL;
> > > +}
> > > +}
> >
> > index is checked for being too large but not for too small ( < 0 )
> > not sure that is intended
> >
> Added a check for (index < 0) to return NULL before the switch, will submit
> in new patch.
> 
> 
> On Sat, Aug 3, 2019 at 12:36 PM Michael Niedermayer 
> wrote:
> 
> > > +if (h->avctx->debug & FF_DEBUG_EXTRACTQP) {
> > > +int mb_height = h->height / 16;
> > > +int mb_width = h->width / 16;
> >
> > has this been tested with files which have dimensions not a multiple of 16
> > ?
> >
> Yes, tested with a 640x360 video, width and height here correspond to the
> coded dimension, which are always multiples of 16 so I believe this should
> not be a problem.
> typedef struct H264Context
> 
> {
> ...
> 
> /* coded dimensions -- 16 * mb w/h */int width
> ,
> height 
> ;
>...
> 
> > +if (!sd) {
> > > +return AVERROR(ENOMEM);
> >
> > buffer leaks here
> >
> I updated it to allocate an array of AVQuantizationParams in the side data
> so that it can all be freed with a single call. I will submit the new patch
> when the patch for libavutil is approved.
> 
> 
> On Sat, Aug 3, 2019 at 12:45 PM Michael Niedermayer 
> wrote:
> 
> >  +int qp_type[AV_QP_ARR_SIZE_AV1];
> 
> What happens if a future codec needs more than AV1 ?

> > i think this would not be extendible without breaking ABI
> 
> Would a macro for largest size be better in this case? I will make that
> change in a new patch also.

a macro would still require a major version bump as its a ABI change, if
it changes.
To fix this would probably require a deeper change, we could also
of course just leave it and accept that as the maximum till the next
ABI major bump

thanks
[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued


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] Extract QP from h264 encoded videos

2019-08-05 Thread Juan De León 
On Sat, Aug 3, 2019 at 12:15 PM Michael Niedermayer 
wrote:

> > +const char* av_get_qp_type_string(enum AVExtractQPSupportedCodecs
> codec_id, int index)
> > +{
> > +switch (codec_id) {
> > +case AV_EXTRACT_QP_CODEC_ID_H264:
> > +return index < AV_QP_ARR_SIZE_H264 ? QP_NAMES_H264[index]
> :NULL;
> > +case AV_EXTRACT_QP_CODEC_ID_VP9:
> > +return index < AV_QP_ARR_SIZE_VP9  ? QP_NAMES_VP9[index]
> :NULL;
> > +case AV_EXTRACT_QP_CODEC_ID_AV1:
> > +return index < AV_QP_ARR_SIZE_AV1  ? QP_NAMES_AV1[index]
> :NULL;
> > +default:
> > +return NULL;
> > +}
> > +}
>
> index is checked for being too large but not for too small ( < 0 )
> not sure that is intended
>
Added a check for (index < 0) to return NULL before the switch, will submit
in new patch.


On Sat, Aug 3, 2019 at 12:36 PM Michael Niedermayer 
wrote:

> > +if (h->avctx->debug & FF_DEBUG_EXTRACTQP) {
> > +int mb_height = h->height / 16;
> > +int mb_width = h->width / 16;
>
> has this been tested with files which have dimensions not a multiple of 16
> ?
>
Yes, tested with a 640x360 video, width and height here correspond to the
coded dimension, which are always multiples of 16 so I believe this should
not be a problem.
typedef struct H264Context

{
...

/* coded dimensions -- 16 * mb w/h */int width
,
height 
;
   ...

> +if (!sd) {
> > +return AVERROR(ENOMEM);
>
> buffer leaks here
>
I updated it to allocate an array of AVQuantizationParams in the side data
so that it can all be freed with a single call. I will submit the new patch
when the patch for libavutil is approved.


On Sat, Aug 3, 2019 at 12:45 PM Michael Niedermayer 
wrote:

>  +int qp_type[AV_QP_ARR_SIZE_AV1];

What happens if a future codec needs more than AV1 ?
> i think this would not be extendible without breaking ABI

Would a macro for largest size be better in this case? I will make that
change in a new patch also.
___
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] Extract QP from h264 encoded videos

2019-08-03 Thread Michael Niedermayer 
On Thu, Aug 01, 2019 at 04:48:32PM -0700, Juan De León wrote:
> Fixed
> 
> design doc: 
> https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing
> 
> Signed-off-by: Juan De León 
> ---
>  libavutil/Makefile  |   2 +
>  libavutil/frame.h   |   6 ++
>  libavutil/quantization_params.c |  42 +
>  libavutil/quantization_params.h | 101 
>  4 files changed, 151 insertions(+)
>  create mode 100644 libavutil/quantization_params.c
>  create mode 100644 libavutil/quantization_params.h
[...]

> +/**
> + * 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
> + * set to 0 for the last block in the array
> + */
> +int w, h;

> +/**
> + * qp array, indexed by type according to
> + * the enum corresponding to the codec
> + */
> +int qp_type[AV_QP_ARR_SIZE_AV1];

What happens if a future codec needs more than AV1 ?
i think this would not be extendible without breaking ABI

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire


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] Extract QP from h264 encoded videos

2019-08-03 Thread Michael Niedermayer 
On Mon, Jul 29, 2019 at 11:12:34AM -0700, Juan De León wrote:
> Here is the second patch, I sent the first one twice accidentaly. 
> First patch is libavutil, this patch is libavcodec.
> 
> Signed-off-by: Juan De León 
> ---
>  libavcodec/avcodec.h   |  1 +
>  libavcodec/h264dec.c   | 44 ++
>  libavcodec/options_table.h |  1 +
>  3 files changed, 46 insertions(+)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index d234271c5b..9e3185720a 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2671,6 +2671,7 @@ typedef struct AVCodecContext {
>  #endif
>  #define FF_DEBUG_BUFFERS 0x8000
>  #define FF_DEBUG_THREADS 0x0001
> +#define FF_DEBUG_EXTRACTQP   0x0002
>  #define FF_DEBUG_GREEN_MD0x0080
>  #define FF_DEBUG_NOMC0x0100
>  
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 8d1bd16a8e..52ad12e55d 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -33,6 +33,7 @@
>  #include "libavutil/opt.h"
>  #include "libavutil/stereo3d.h"
>  #include "libavutil/timer.h"
> +#include "libavutil/quantization_params.h"
>  #include "internal.h"
>  #include "bytestream.h"
>  #include "cabac.h"
> @@ -922,6 +923,49 @@ static int finalize_frame(H264Context *h, AVFrame *dst, 
> H264Picture *out, int *g
>  }
>  }
>  
> +if (h->avctx->debug & FF_DEBUG_EXTRACTQP) {
> +int mb_height = h->height / 16;
> +int mb_width = h->width / 16;

has this been tested with files which have dimensions not a multiple of 16 ?


> +int mb_xy = mb_width * mb_height;
> +
> +int buf_size = sizeof(AVQuantizationParamsArray) +
> +   mb_xy * sizeof(AVQuantizationParams);
> +AVBufferRef *buffer = av_buffer_alloc(buf_size);
> +if (!buffer) {
> +return AVERROR(ENOMEM);
> +}
> +
> +AVQuantizationParamsArray *params = (AVQuantizationParamsArray 
> *)buffer->data;
> +params->nb_blocks = mb_xy;
> +params->codec_id = h->avctx->codec_id;
> +// offset memory for qp_arr in same buffer
> +params->qp_arr = (AVQuantizationParams*) (params + 1);
> +
> +// loop allocate qp
> +int qp_index = 0;
> +for (int mb_y = 0; mb_y < mb_height; mb_y++) {
> +for (int mb_x = 0; mb_x < mb_width; mb_x++) {
> +int qs_index = mb_x + mb_y * h->mb_stride;
> +AVQuantizationParams *qp_block = &(params->qp_arr[qp_index]);
> +
> +qp_block->x = mb_x * 16;
> +qp_block->y = mb_y * 16;
> +qp_block->w = qp_block->h = 16;
> +qp_block->type[QP_H264] = out->qscale_table[qs_index];
> +
> +qp_index++;
> +}
> +}
> +

> +AVFrameSideData *sd;
> +sd = av_frame_new_side_data_from_buf(dst,
> + 
> AV_FRAME_DATA_QUANTIZATION_PARAMS,
> + buffer);
> +if (!sd) {
> +return AVERROR(ENOMEM);

buffer leaks here


[...]

thx

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Elect your leaders based on what they did after the last election, not
based on what they say before an election.



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] Extract QP from h264 encoded videos

2019-08-03 Thread Michael Niedermayer 
On Thu, Aug 01, 2019 at 04:48:32PM -0700, Juan De León wrote:
> Fixed

That would end in the commit message which is probably unintended


> 
> design doc: 
> https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing
> 
> Signed-off-by: Juan De León 
> ---
>  libavutil/Makefile  |   2 +
>  libavutil/frame.h   |   6 ++
>  libavutil/quantization_params.c |  42 +
>  libavutil/quantization_params.h | 101 
>  4 files changed, 151 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..be1a9c3a9c 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -60,6 +60,7 @@ HEADERS = adler32.h 
> \
>pixdesc.h \
>pixelutils.h  \
>pixfmt.h  \
> +  quantization_params.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..b64fd9c02c 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 is stored as AVQuantizationParamsArray type, described in
> + * libavuitl/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 00..152be71ba7
> --- /dev/null
> +++ b/libavutil/quantization_params.c
> @@ -0,0 +1,42 @@
> +/*
> + * 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 
> +
> +#include "libavutil/quantization_params.h"
> +

> +static const char* const QP_NAMES_H264[] = {"qp", "qpcb", "qpcr"};
> +
> +static const char* const QP_NAMES_VP9[] = {"qyac", "qydc", "quvdc", "quvac",
> +   "qiyac", "qiydc", "qiuvdc", 
> "qiuvac"};
> +
> +static const char* const QP_NAMES_AV1[] = {"qyac", "qydc", "qudc", "quac", 
> "qvdc", "qvac",
> +  "qiyac", "qiydc", "qiudc", "qiuac", 
> "qivdc", "qivac"};
> +

The meaning of the chars in the strings should maybe be described
So generic code could interpret a new codecs qp values
though for such generic interpretation more would be needed like which direction
quality improves, if its linear or log scale, ...
so maybe this is just out of scope of this initial implementation


> +const char* av_get_qp_type_string(enum AVExtractQPSupportedCodecs codec_id, 
> int index)
> +{
> +switch (codec_id) {
> +case AV_EXTRACT_QP_CODEC_ID_H264:
> +return index < AV_QP_ARR_SIZE_H264 ? QP_NAMES_H264[index] :NULL;
> +case AV_EXTRACT_QP_CODEC_ID_VP9:
> +return index < AV_QP_ARR_SIZE_VP9  ? QP_NAMES_VP9[index]  :NULL;
> +case

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

2019-08-01 Thread Michael Niedermayer 
On Tue, Jul 30, 2019 at 06:47:23PM -0700, Juan De León wrote:
> Removed AVQuantizationParamsArray to prevent ambiguous memory allocation.
> For simplicity, the side data will be allocated as an array of 
> AVQuantizationParams and the last element of the array will have w and h set 
> to 0.
> 
> Better explained in the doc.
> design doc: 
> https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing
> 
> Signed-off-by: Juan De León 
> ---
>  libavutil/Makefile  |   2 +
>  libavutil/frame.h   |   6 ++
>  libavutil/quantization_params.c |  41 +
>  libavutil/quantization_params.h | 101 
>  4 files changed, 150 insertions(+)
>  create mode 100644 libavutil/quantization_params.c
>  create mode 100644 libavutil/quantization_params.h

doesnt build here

libavutil/quantization_params.c: In function ‘av_get_qp_type_string’:
libavutil/quantization_params.c:33:72: error: ‘NULL’ undeclared (first use in 
this function)
 return index < AV_QP_ARR_SIZE_H264 ? QP_NAMES_H264[index] :NULL;
^
libavutil/quantization_params.c:33:72: note: each undeclared identifier is 
reported only once for each function it appears in
libavutil/quantization_params.c:41:1: error: control reaches end of non-void 
function [-Werror=return-type]
 }
 ^
cc1: some warnings being treated as errors
make: *** [libavutil/quantization_params.o] Error 1

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus


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] Extract QP from h264 encoded videos

2019-08-01 Thread Juan De León 
On Tue, Jul 30, 2019 at 6:50 PM Juan De León  wrote:

> Removed AVQuantizationParamsArray to prevent ambiguous memory allocation.
> For simplicity, the side data will be allocated as an array of
> AVQuantizationParams and the last element of the array will have w and h
> set to 0.
>
> Better explained in the doc.
> design doc:
> https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing
>
> Signed-off-by: Juan De León 
> ---
>  libavutil/Makefile  |   2 +
>  libavutil/frame.h   |   6 ++
>  libavutil/quantization_params.c |  41 +
>  libavutil/quantization_params.h | 101 
>  4 files changed, 150 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..be1a9c3a9c 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -60,6 +60,7 @@ HEADERS = adler32.h
>\
>pixdesc.h \
>pixelutils.h  \
>pixfmt.h  \
> +  quantization_params.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..b64fd9c02c 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 is stored as AVQuantizationParamsArray type, described in
> + * libavuitl/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 00..7d8b0a4526
> --- /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", "qpcb", "qpcr"};
> +
> +static const char* const QP_NAMES_VP9[] = {"qyac", "qydc", "quvdc",
> "quvac",
> +   "qiyac", "qiydc", "qiuvdc",
> "qiuvac"};
> +
> +static const char* const QP_NAMES_AV1[] = {"qyac", "qydc", "qudc",
> "quac", "qvdc", "qvac",
> +  "qiyac", "qiydc", "qiudc", "qiuac",
> "qivdc", "qivac"};
> +
> +const char* av_get_qp_type_string(enum AVExtractQPSupportedCodecs
> codec_id, int index)
> +{
> +switch (codec_id) {
> +case AV_EXTRACT_QP_CODEC_ID_H264:
> +return index < AV_QP_ARR_SIZE_H264 ? QP_NAMES_H264[index]
> :NULL;
> +case AV_EXTRACT_QP_CODEC_ID_VP9:
> +return index < AV_QP_ARR_SIZE_VP9  ? QP_NAMES_VP9[index]
> :NULL;
> +case AV_EXTRACT_QP_CODEC_ID_AV1:
> +return index < AV_QP_ARR_SIZE_AV1  ? QP_NAMES_AV1[index]
> :NULL;
> +default:
> +return NULL;
> +}
> +}
> diff --git a/libavutil/quantization_params.h
> b/libavutil/quantization_params.h
> new file

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

2019-07-30 Thread Michael Niedermayer 
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  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
[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

You can kill me, but you cannot change the truth.


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] Extract QP from h264 encoded videos

2019-07-30 Thread Juan De León 
On Tue, Jul 30, 2019 at 2:20 AM Andrey Semashev 
wrote:

> Just a thought. If you need codec ids and implement codec-specific
> functionality, then this whole thing probably belongs to libavcodec, not
> libavutil.
>

My understanding is that only encoders and decoders are supposed to be in
libavcodec.
I believe this fits better in libavutil for consistency.
___
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] Extract QP from h264 encoded videos

2019-07-30 Thread Juan De León 
On Mon, Jul 29, 2019 at 7:59 PM James Almer  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.
___
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] Extract QP from h264 encoded videos

2019-07-30 Thread Andrey Semashev 

On 7/30/19 12:11 PM, Andrey Semashev wrote:

On 7/30/19 5:39 AM, Juan De León wrote:
I tried to fix all you suggested, please have a look and let me know 
what you think.


design doc: 
https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing 



Signed-off-by: Juan De León 
---
  libavutil/Makefile  |   2 +
  libavutil/frame.h   |   6 ++
  libavutil/quantization_params.c |  42 
  libavutil/quantization_params.h | 114 
  4 files changed, 164 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..be1a9c3a9c 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -60,6 +60,7 @@ HEADERS = 
adler32.h \

pixdesc.h \

pixelutils.h  \

pixfmt.h  \
+  
quantization_params.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..b64fd9c02c 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 is stored as AVQuantizationParamsArray type, 
described in

+ * libavuitl/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 00..fc51b55eee
--- /dev/null
+++ b/libavutil/quantization_params.c
@@ -0,0 +1,42 @@
+/*
+ * 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"
+#include "libavutil/mem.h"
+
+static const char* const QP_NAMES_H264[] = {"qpy", "qpuv"};
+
+static const char* const QP_NAMES_VP9[] = {"qyac", "qydc", "quvdc", 
"quvac",
+   "qiyac", "qiydc", 
"qiuvdc", "qiuvac"};

+
+static const char* const QP_NAMES_AV1[] = {"qydc", "qyac", "qudc", 
"quac", "qvdc", "qvac",
+  "qiydc", "qiyac", "qiudc", 
"qiuac", "qivdc", "qivac"};

+
+char* av_get_qp_type_string(enum AVExtractQPSupportedCodecs codec_id, 
int index)

+{
+    switch (codec_id) {
+    case AV_EXTRACT_QP_CODEC_ID_H264:
+    return index < AV_QP_ARR_SIZE_H264 ? 
av_strdup(QP_NAMES_H264[index]) :NULL;

+    case AV_EXTRACT_QP_CODEC_ID_VP9:
+    return index < AV_QP_ARR_SIZE_VP9  ? 
av_strdup(QP_NAMES_VP9[index])  :NULL;

+    case AV_EXTRACT_QP_CODEC_ID_AV1:
+    return index < AV_QP_ARR_SIZE_AV1  ? 
av_strdup(QP_NAMES_AV1[index])  :NULL;

+    default:
+    return NULL;
+    }
+}


Why strdup here? Why not return the pointer from the static array?

If you really do intend to strdup, the function documentation should 
state clearly that the caller is supposed to av_free the returned pointer.


diff --git a/libavutil/quantization_params.h

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

2019-07-30 Thread Andrey Semashev 

On 7/30/19 5:39 AM, Juan De León wrote:

I tried to fix all you suggested, please have a look and let me know what you 
think.

design doc: 
https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing

Signed-off-by: Juan De León 
---
  libavutil/Makefile  |   2 +
  libavutil/frame.h   |   6 ++
  libavutil/quantization_params.c |  42 
  libavutil/quantization_params.h | 114 
  4 files changed, 164 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..be1a9c3a9c 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -60,6 +60,7 @@ HEADERS = adler32.h   
  \
pixdesc.h \
pixelutils.h  \
pixfmt.h  \
+  quantization_params.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..b64fd9c02c 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 is stored as AVQuantizationParamsArray type, described in
+ * libavuitl/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 00..fc51b55eee
--- /dev/null
+++ b/libavutil/quantization_params.c
@@ -0,0 +1,42 @@
+/*
+ * 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"
+#include "libavutil/mem.h"
+
+static const char* const QP_NAMES_H264[] = {"qpy", "qpuv"};
+
+static const char* const QP_NAMES_VP9[] = {"qyac", "qydc", "quvdc", "quvac",
+   "qiyac", "qiydc", "qiuvdc", 
"qiuvac"};
+
+static const char* const QP_NAMES_AV1[] = {"qydc", "qyac", "qudc", "quac", "qvdc", 
"qvac",
+  "qiydc", "qiyac", "qiudc", "qiuac", "qivdc", 
"qivac"};
+
+char* av_get_qp_type_string(enum AVExtractQPSupportedCodecs codec_id, int 
index)
+{
+switch (codec_id) {
+case AV_EXTRACT_QP_CODEC_ID_H264:
+return index < AV_QP_ARR_SIZE_H264 ? 
av_strdup(QP_NAMES_H264[index]) :NULL;
+case AV_EXTRACT_QP_CODEC_ID_VP9:
+return index < AV_QP_ARR_SIZE_VP9  ? 
av_strdup(QP_NAMES_VP9[index])  :NULL;
+case AV_EXTRACT_QP_CODEC_ID_AV1:
+return index < AV_QP_ARR_SIZE_AV1  ? 
av_strdup(QP_NAMES_AV1[index])  :NULL;
+default:
+return NULL;
+}
+}
diff --git a/libavutil/quantization_params.h b/libavutil/quantization_params.h
new file mode 100644
index 00..d123aade3b
--- /dev/null
+++ b/libavutil/quantization_params.h
@@ -0,0 +1,114 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

2019-07-29 Thread James Almer 
On 7/29/2019 11:19 PM, Juan De León wrote:
> On Mon, Jul 29, 2019 at 12:48 PM Mark Thompson  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
> 
> 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.
___
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] Extract QP from h264 encoded videos

2019-07-29 Thread Juan De León 
On Mon, Jul 29, 2019 at 12:48 PM Mark Thompson  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

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
___
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] Extract QP from h264 encoded videos

2019-07-29 Thread Mark Thompson 
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 
> ---
>  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 00..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 00..7a3daeaae5
> --- /dev/null
> +++ b/libavutil/quantization_params.h
> @@ -0,0 +1,106

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

2019-07-29 Thread Juan De León 
Thank you for the feedback Andrey, I will fix it ASAP.

On Mon, Jul 29, 2019 at 12:11 PM Andreas Håkon 
wrote:

> Interesting patch. But I have a question:
> Could you implement the same thing when using the hardware decoders?

I believe this might be in the scope of my project.
I'm not too familiar with hardware decoders but when I complete the rest of
the implementation I will take a look at this.


> And also, could you make a similar patch for MPEG-2 as well?

The scope was limited to H264, VP9 and AV1 but I will talk about it with my
team.
Again, thanks for the feedback.
___
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] Extract QP from h264 encoded videos

2019-07-29 Thread Andreas Håkon 
Hi Juan,


‐‐‐ Original Message ‐‐‐
On Monday, 29 de July de 2019 20:12, Juan De León 
 wrote:

> Here is the second patch, I sent the first one twice accidentaly.
> First patch is libavutil, this patch is libavcodec.
>

Interesting patch. But I have a question:
Could you implement the same thing when using the hardware decoders?

And also, could you make a similar patch for MPEG-2 as well?

Regards.
A.H.

---

___
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] Extract QP from h264 encoded videos

2019-07-29 Thread Andrey Semashev 

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 
---
  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 00..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 00..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

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

2019-07-25 Thread Michael Niedermayer 
On Wed, Jul 24, 2019 at 12:18:59PM -0700, Juan De León wrote:
> ---
>  libavcodec/avcodec.h|   1 +
>  libavcodec/h264dec.c|  37 
>  libavcodec/options_table.h  |   1 +

>  libavutil/Makefile  |   2 +
>  libavutil/frame.h   |   6 ++
>  libavutil/quantization_params.c |  40 +
>  libavutil/quantization_params.h | 102 

the changes to libavutil and libavcodec should be in separate patches


>  7 files changed, 189 insertions(+)
>  create mode 100644 libavutil/quantization_params.c
>  create mode 100644 libavutil/quantization_params.h
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index d234271c5b..9e3185720a 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2671,6 +2671,7 @@ typedef struct AVCodecContext {
>  #endif
>  #define FF_DEBUG_BUFFERS 0x8000
>  #define FF_DEBUG_THREADS 0x0001
> +#define FF_DEBUG_EXTRACTQP   0x0002
>  #define FF_DEBUG_GREEN_MD0x0080
>  #define FF_DEBUG_NOMC0x0100
>  
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 8d1bd16a8e..07b85f4e0a 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -33,6 +33,7 @@
>  #include "libavutil/opt.h"
>  #include "libavutil/stereo3d.h"
>  #include "libavutil/timer.h"
> +#include "libavutil/quantization_params.h"
>  #include "internal.h"
>  #include "bytestream.h"
>  #include "cabac.h"
> @@ -922,6 +923,42 @@ static int finalize_frame(H264Context *h, AVFrame *dst, 
> H264Picture *out, int *g
>  }
>  }
>  
> +if (h->avctx->debug & FF_DEBUG_EXTRACTQP) {
> +int mb_height = h->height / 16;
> +int mb_width = h->width / 16;
> +int mb_xy = mb_width * mb_height;
> +
> +AVFrameSideData *sd;
> +sd = av_frame_new_side_data(dst, AV_FRAME_DATA_QUANTIZATION_PARAMS,
> +  sizeof(AVQuantizationParamsArray));
> +
> +AVQuantizationParamsArray *params;
> +params = (AVQuantizationParamsArray *)sd->data;
> +params->nb_blocks = mb_xy;
> +params->qp_arr = av_malloc_array(mb_xy, 
> sizeof(AVQuantizationParams));
> +
> +params->codec_id = h->avctx->codec_id;

missing failure checks for av_frame_new_side_data and av_malloc_array


> +
> +// loop allocate QP
> +int qp_index = 0;
> +for (int mb_y = 0; mb_y < mb_height; mb_y++) {
> +for (int mb_x = 0; mb_x < mb_width; mb_x++) {
> +int qs_index = mb_x + mb_y * h->mb_stride;
> +AVQuantizationParams *qp_block = &(params->qp_arr[qp_index]);
> +
> +qp_block->x = mb_x * 16;
> +qp_block->y = mb_y * 16;
> +qp_block->w = qp_block->h = 16;
> +
> +// ALLOCATE MEMORY TO THE QP ARRAY
> +qp_block->type = av_malloc(QP_TYPE_ARR_SIZE_H264 * 
> sizeof(int));
> +qp_block->type[QP_H264] = out->qscale_table[qs_index];
> +
> +qp_index++;
> +}
> +}
> +}
> +
>  return 0;
>  }
>  
> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> index 4a266eca16..e0e78a69c5 100644
> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -219,6 +219,7 @@ static const AVOption avcodec_options[] = {
>  {"buffers", "picture buffer allocations", 0, AV_OPT_TYPE_CONST, {.i64 = 
> FF_DEBUG_BUFFERS }, INT_MIN, INT_MAX, V|D, "debug"},
>  {"thread_ops", "threading operations", 0, AV_OPT_TYPE_CONST, {.i64 = 
> FF_DEBUG_THREADS }, INT_MIN, INT_MAX, V|A|D, "debug"},
>  {"nomc", "skip motion compensation", 0, AV_OPT_TYPE_CONST, {.i64 = 
> FF_DEBUG_NOMC }, INT_MIN, INT_MAX, V|A|D, "debug"},
> +{"extractqp", "enable QP extraction per frame", 0, AV_OPT_TYPE_CONST, {.i64 
> = FF_DEBUG_EXTRACTQP }, INT_MIN, INT_MAX, V|D, "debug"},
>  {"dia_size", "diamond type & size for motion estimation", OFFSET(dia_size), 
> AV_OPT_TYPE_INT, {.i64 = DEFAULT }, INT_MIN, INT_MAX, V|E},
>  {"last_pred", "amount of motion predictors from the previous frame", 
> OFFSET(last_predictor_count), AV_OPT_TYPE_INT, {.i64 = DEFAULT }, INT_MIN, 
> INT_MAX, V|E},
>  #if FF_API_PRIVATE_OPT
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 8a7a44e4b5..be1a9c3a9c 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -60,6 +60,7 @@ HEADERS = adler32.h 
> \
>pixdesc.h \
>pixelutils.h  \
>pixfmt.h  \
> +  quantization_params.h \
>random_seed.h \
>rc4.h \
>rational.h

Re: [FFmpeg-devel] [PATCH] Extract QP from h264 encoded videos

2019-07-24 Thread Juan De León 
Previous thread:
http://ffmpeg.org/pipermail/ffmpeg-devel/2019-July/246951.html
I added the modifications to the decoder, I ran some tests for performance
and run times are only affected if my flag is enabled.
Decoded 3 different encoded videos 20 times each with and without my debug
flag, here are the results:

*ExtractQP disabled:*
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug 0 -i tveryfast.mp4 -f null -
MeanStd.Dev.Min Median  Max
real0.747   0.007   0.735   0.749   0.756
user5.582   0.025   5.547   5.574   5.627
sys 0.166   0.028   0.120   0.167   0.224
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug 0 -i tmedium.mp4 -f null -
MeanStd.Dev.Min Median  Max
real0.865   0.009   0.845   0.864   0.887
user6.296   0.036   6.198   6.299   6.365
sys 0.195   0.026   0.142   0.199   0.247
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug 0 -i tveryslow.mp4 -f null -
MeanStd.Dev.Min Median  Max
real0.919   0.011   0.892   0.920   0.943
user6.398   0.042   6.311   6.381   6.476
sys 0.229   0.032   0.169   0.238   0.287

*ExtractQP enabled: *
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug extractqp -i tveryfast.mp4 -f
null -
MeanStd.Dev.Min Median  Max
real1.126   0.032   1.076   1.132   1.216
user6.433   0.054   6.347   6.430   6.561
sys 1.069   0.047   0.989   1.063   1.161
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug extractqp -i tmedium.mp4 -f
null -
MeanStd.Dev.Min Median  Max
real1.178   0.020   1.143   1.176   1.217
user7.091   0.055   7.020   7.081   7.196
sys 1.031   0.057   0.898   1.043   1.131
===> multitime results
1: ffmpeg -hide_banner -loglevel panic -debug extractqp -i tveryslow.mp4 -f
null -
MeanStd.Dev.Min Median  Max
real1.234   0.028   1.196   1.230   1.322
user7.212   0.077   6.996   7.230   7.345
sys 1.067   0.076   0.938   1.062   1.283
___
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".