On Sat, Aug 3, 2019 at 12:15 PM Michael Niedermayer <mich...@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 <mich...@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 <mich...@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. _______________________________________________ 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".