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

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

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190805/ededef6b/attachment.sig>