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

[Juan De León]

On Sat, Aug 3, 2019 at 12:15 PM Michael Niedermayer <michael at niedermayer.cc>
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 <michael at niedermayer.cc>
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
<https://cs.corp.google.com/piper///depot/google3/third_party/ffmpeg/src/libavcodec/h264dec.h?l=337&gs=kythe%253A%252F%252Fgoogle3%253Flang%253Dc%25252B%25252B%253Fpath%253Dthird_party%252Fffmpeg%252Fsrc%252Flibavcodec%252Fh264dec.h%2523H264Context%252523c%252523dEG_os146C2&gsn=H264Context&ct=xref_usages>
{
...

    /* coded dimensions -- 16 * mb w/h */    int width
<https://cs.corp.google.com/piper///depot/google3/third_party/ffmpeg/src/libavcodec/h264dec.h?l=359&gs=kythe%253A%252F%252Fgoogle3%253Flang%253Dc%25252B%25252B%253Fpath%253Dthird_party%252Fffmpeg%252Fsrc%252Flibavcodec%252Fh264dec.h%2523ZImaFPq8HKEGWVaoII7Cf0AKkOcO2Z5yD-AjKA2sPy0&gsn=width&ct=xref_usages>,
height <https://cs.corp.google.com/piper///depot/google3/third_party/ffmpeg/src/libavcodec/h264dec.h?l=359&gs=kythe%253A%252F%252Fgoogle3%253Flang%253Dc%25252B%25252B%253Fpath%253Dthird_party%252Fffmpeg%252Fsrc%252Flibavcodec%252Fh264dec.h%2523-tOCyFteI1_t9m4iN9IbhvTDxCRW6aBjm8eEw4zgfno&gsn=height&ct=xref_usages>;
   ...

> +        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 <michael at niedermayer.cc>
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.