[FFmpeg-devel] [PATCH] Add support for RockChip Media Process Platform

LongChair . LongChair at hotmail.com
Tue Sep 5 09:45:45 EEST 2017



Le 03/09/2017 à 14:51, Mark Thompson a écrit :
> On 01/09/17 16:45, LongChair . wrote:
>> From: LongChair <LongChair at hotmail.com>
>>
>> Add support for RockChip Media Process Platform This adds hardware decoding for h264 / HEVC / VP8 using MPP Rockchip API. Will return frames holding an AVDRMFrameDescriptor struct in buf[0] that allows drm / dmabuf usage. Was tested on RK3288 (...
> Some linebreaks in the commit message, please.
Yes will be fixed in next commit
>
>> ---
>>   Changelog              |   1 +
>>   configure              |  13 +-
>>   libavcodec/Makefile    |   3 +
>>   libavcodec/allcodecs.c |   6 +
>>   libavcodec/rkmppdec.c  | 537 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   5 files changed, 559 insertions(+), 1 deletion(-)
>>   create mode 100644 libavcodec/rkmppdec.c
> This configures but doesn't build with the released version of the MPP library that I had by default (1.3.1 from Debian/Linaro somehow?):
>
> """
> CC      libavcodec/rkmppdec.o
> src/libavcodec/rkmppdec.c: In function ‘ffrkmpp_receive_frame’:
> src/libavcodec/rkmppdec.c:455:51: error: ‘MPP_DEC_GET_FREE_PACKET_SLOT_COUNT’ undeclared (first use in this function)
>           ret = decoder->mpi->control(decoder->ctx, MPP_DEC_GET_FREE_PACKET_SLOT_COUNT, &freeslots);
>                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> """
>
> I think it needs an additional configure test to ensure that the version is usable.  I installed some git version which calls itself 1.3.0 and it works, so it may need to be a feature test rather than a version one...
Right, as there is no good version management in the rockchip MPP 
pkgconfig file, i'll add a check for MPP_DEC_GET_FREE_PACKET_SLOT_COUNT 
in the appropriate header because this is the one change that we had to 
upstream into MPP for ffmpeg
>
>> diff --git a/Changelog b/Changelog
>> index 1dfb8b5..c19c1d0 100644
>> --- a/Changelog
>> +++ b/Changelog
>> @@ -70,6 +70,7 @@ version 3.3:
>>   - Removed asyncts filter (use af_aresample instead)
>>   - Intel QSV-accelerated VP8 video decoding
>>   - VAAPI-accelerated deinterlacing
>> +- Addition of Rockchip MPP harware decoding
> This is in the wrong place, it should be in the "next" list rather than the list for 3.3.
>
> Also, typo "harware".
Both Fixed.
>
>>   version 3.2:
>> diff --git a/configure b/configure
>> index ebd561d..62fbcca 100755
>> --- a/configure
>> +++ b/configure
>> @@ -307,6 +307,7 @@ External library support:
>>     --disable-nvenc          disable Nvidia video encoding code [autodetect]
>>     --enable-omx             enable OpenMAX IL code [no]
>>     --enable-omx-rpi         enable OpenMAX IL code for Raspberry Pi [no]
>> +  --enable-rkmpp           enable Rockchip Media Process Platform code [no]
>>     --disable-vaapi          disable Video Acceleration API (mainly Unix/Intel) code [autodetect]
>>     --disable-vda            disable Apple Video Decode Acceleration code [autodetect]
>>     --disable-vdpau          disable Nvidia Video Decode and Presentation API for Unix code [autodetect]
>> @@ -1614,6 +1615,7 @@ HWACCEL_LIBRARY_LIST="
>>       libmfx
>>       mmal
>>       omx
>> +    rkmpp
>>   "
>>   
>>   DOCUMENT_LIST="
>> @@ -2627,6 +2629,7 @@ h264_dxva2_hwaccel_select="h264_decoder"
>>   h264_mediacodec_hwaccel_deps="mediacodec"
>>   h264_mmal_hwaccel_deps="mmal"
>>   h264_qsv_hwaccel_deps="libmfx"
>> +h264_rkmpp_hwaccel_deps="rkmpp">  h264_vaapi_hwaccel_deps="vaapi"
>>   h264_vaapi_hwaccel_select="h264_decoder"
>>   h264_vda_hwaccel_deps="vda"
>> @@ -2647,6 +2650,7 @@ hevc_d3d11va2_hwaccel_select="hevc_decoder"
>>   hevc_dxva2_hwaccel_deps="dxva2 DXVA_PicParams_HEVC"
>>   hevc_dxva2_hwaccel_select="hevc_decoder"
>>   hevc_qsv_hwaccel_deps="libmfx"
>> +hevc_rkmpp_hwaccel_deps="rkmpp"
>>   hevc_vaapi_hwaccel_deps="vaapi VAPictureParameterBufferHEVC"
>>   hevc_vaapi_hwaccel_select="hevc_decoder"
>>   hevc_vdpau_hwaccel_deps="vdpau VdpPictureInfoHEVC"
>> @@ -2714,6 +2718,7 @@ vp9_cuvid_hwaccel_deps="cuda cuvid"
>>   vp9_cuvid_hwaccel_select="vp9_cuvid_decoder"
>>   vp8_mediacodec_hwaccel_deps="mediacodec"
>>   vp8_qsv_hwaccel_deps="libmfx"
>> +vp8_rkmpp_hwaccel_deps="rkmpp"
>>   vp9_d3d11va_hwaccel_deps="d3d11va DXVA_PicParams_VP9"
>>   vp9_d3d11va_hwaccel_select="vp9_decoder"
>>   vp9_d3d11va2_hwaccel_deps="d3d11va DXVA_PicParams_VP9"
>> @@ -2757,6 +2762,8 @@ h264_qsv_decoder_deps="libmfx"
>>   h264_qsv_decoder_select="h264_mp4toannexb_bsf h264_parser qsvdec h264_qsv_hwaccel"
>>   h264_qsv_encoder_deps="libmfx"
>>   h264_qsv_encoder_select="qsvenc"
>> +h264_rkmpp_decoder_deps="rkmpp"
>> +h264_rkmpp_decoder_select="h264_mp4toannexb_bsf rkmpp h264_rkmpp_hwaccel"
>>   h264_vaapi_encoder_deps="VAEncPictureParameterBufferH264"
>>   h264_vaapi_encoder_select="vaapi_encode golomb"
>>   h264_vda_decoder_deps="vda"
>> @@ -2772,6 +2779,8 @@ hevc_qsv_decoder_deps="libmfx"
>>   hevc_qsv_decoder_select="hevc_mp4toannexb_bsf hevc_parser qsvdec hevc_qsv_hwaccel"
>>   hevc_qsv_encoder_deps="libmfx"
>>   hevc_qsv_encoder_select="hevcparse qsvenc"
>> +hevc_rkmpp_decoder_deps="rkmpp"
>> +hevc_rkmpp_decoder_select="hevc_mp4toannexb_bsf rkmpp hevc_rkmpp_hwaccel"
>>   hevc_vaapi_encoder_deps="VAEncPictureParameterBufferHEVC"
>>   hevc_vaapi_encoder_select="vaapi_encode golomb"
>>   mjpeg_cuvid_decoder_deps="cuda cuvid"
>> @@ -2811,6 +2820,8 @@ vp8_cuvid_decoder_deps="cuda cuvid"
>>   vp8_mediacodec_decoder_deps="mediacodec"
>>   vp8_qsv_decoder_deps="libmfx"
>>   vp8_qsv_decoder_select="qsvdec vp8_qsv_hwaccel vp8_parser"
>> +vp8_rkmpp_decoder_deps="rkmpp"
>> +vp8_rkmpp_decoder_select="rkmpp vp8_rkmpp_hwaccel"
> Just have a decoder in each case.  The dummy hwaccels don't do anything and are not necessary.
removed
>>   vp8_vaapi_encoder_deps="VAEncPictureParameterBufferVP8"
>>   vp8_vaapi_encoder_select="vaapi_encode"
>>   vp9_cuvid_decoder_deps="cuda cuvid"
>> @@ -6008,7 +6019,7 @@ enabled openssl           && { use_pkg_config openssl openssl/ssl.h OPENSSL_init
>>                                  check_lib openssl openssl/ssl.h SSL_library_init -lssl -lcrypto -lws2_32 -lgdi32 ||
>>                                  die "ERROR: openssl not found"; }
>>   enabled qtkit_indev      && { check_header_objcc QTKit/QTKit.h || disable qtkit_indev; }
>> -
>> +enabled rkmpp             && { check_lib rkmpp rockchip/rk_mpi.h mpp_create "-lrockchip_mpp" || die "ERROR : Rockchip MPP was not found."; }
> The library has pkgconfig, please use it.
added pkgconfig requirement for rockchip_mpp.
>
>>   if enabled gcrypt; then
>>       GCRYPT_CONFIG="${cross_prefix}libgcrypt-config"
>>       if "${GCRYPT_CONFIG}" --version > /dev/null 2>&1; then
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index 982d7f5..b75912f 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -336,6 +336,7 @@ OBJS-$(CONFIG_H264_VDA_DECODER)        += vda_h264_dec.o
>>   OBJS-$(CONFIG_H264_OMX_ENCODER)        += omx.o
>>   OBJS-$(CONFIG_H264_QSV_DECODER)        += qsvdec_h2645.o
>>   OBJS-$(CONFIG_H264_QSV_ENCODER)        += qsvenc_h264.o
>> +OBJS-$(CONFIG_H264_RKMPP_DECODER)      += rkmppdec.o
>>   OBJS-$(CONFIG_H264_VAAPI_ENCODER)      += vaapi_encode_h264.o vaapi_encode_h26x.o
>>   OBJS-$(CONFIG_H264_VIDEOTOOLBOX_ENCODER) += videotoolboxenc.o
>>   OBJS-$(CONFIG_HAP_DECODER)             += hapdec.o hap.o
>> @@ -350,6 +351,7 @@ OBJS-$(CONFIG_NVENC_HEVC_ENCODER)      += nvenc_hevc.o
>>   OBJS-$(CONFIG_HEVC_QSV_DECODER)        += qsvdec_h2645.o
>>   OBJS-$(CONFIG_HEVC_QSV_ENCODER)        += qsvenc_hevc.o hevc_ps_enc.o       \
>>                                             hevc_data.o
>> +OBJS-$(CONFIG_HEVC_RKMPP_DECODER)      += rkmppdec.o
>>   OBJS-$(CONFIG_HEVC_VAAPI_ENCODER)      += vaapi_encode_h265.o vaapi_encode_h26x.o
>>   OBJS-$(CONFIG_HNM4_VIDEO_DECODER)      += hnm4video.o
>>   OBJS-$(CONFIG_HQ_HQA_DECODER)          += hq_hqa.o hq_hqadata.o hq_hqadsp.o \
>> @@ -621,6 +623,7 @@ OBJS-$(CONFIG_VP8_DECODER)             += vp8.o vp56rac.o
>>   OBJS-$(CONFIG_VP8_CUVID_DECODER)       += cuvid.o
>>   OBJS-$(CONFIG_VP8_MEDIACODEC_DECODER)  += mediacodecdec.o
>>   OBJS-$(CONFIG_VP8_QSV_DECODER)         += qsvdec_other.o
>> +OBJS-$(CONFIG_VP8_RKMPP_DECODER)       += rkmppdec.o
>>   OBJS-$(CONFIG_VP8_VAAPI_ENCODER)       += vaapi_encode_vp8.o
>>   OBJS-$(CONFIG_VP9_DECODER)             += vp9.o vp9data.o vp9dsp.o vp9lpf.o vp9recon.o \
>>                                             vp9block.o vp9prob.o vp9mvs.o vp56rac.o \
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index 1e5942d..d44666f 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -71,6 +71,7 @@ static void register_all(void)
>>       REGISTER_HWACCEL(H264_MEDIACODEC,   h264_mediacodec);
>>       REGISTER_HWACCEL(H264_MMAL,         h264_mmal);
>>       REGISTER_HWACCEL(H264_QSV,          h264_qsv);
>> +    REGISTER_HWACCEL(H264_RKMPP,        h264_rkmpp);
>>       REGISTER_HWACCEL(H264_VAAPI,        h264_vaapi);
>>       REGISTER_HWACCEL(H264_VDA,          h264_vda);
>>       REGISTER_HWACCEL(H264_VDA_OLD,      h264_vda_old);
>> @@ -82,6 +83,7 @@ static void register_all(void)
>>       REGISTER_HWACCEL(HEVC_DXVA2,        hevc_dxva2);
>>       REGISTER_HWACCEL(HEVC_MEDIACODEC,   hevc_mediacodec);
>>       REGISTER_HWACCEL(HEVC_QSV,          hevc_qsv);
>> +    REGISTER_HWACCEL(HEVC_RKMPP,        hevc_rkmpp);
>>       REGISTER_HWACCEL(HEVC_VAAPI,        hevc_vaapi);
>>       REGISTER_HWACCEL(HEVC_VDPAU,        hevc_vdpau);
>>       REGISTER_HWACCEL(MJPEG_CUVID,       mjpeg_cuvid);
>> @@ -117,6 +119,7 @@ static void register_all(void)
>>       REGISTER_HWACCEL(VP8_CUVID,         vp8_cuvid);
>>       REGISTER_HWACCEL(VP8_MEDIACODEC,    vp8_mediacodec);
>>       REGISTER_HWACCEL(VP8_QSV,           vp8_qsv);
>> +    REGISTER_HWACCEL(VP8_RKMPP,         vp8_rkmpp);
>>       REGISTER_HWACCEL(VP9_CUVID,         vp9_cuvid);
>>       REGISTER_HWACCEL(VP9_D3D11VA,       vp9_d3d11va);
>>       REGISTER_HWACCEL(VP9_D3D11VA2,      vp9_d3d11va2);
>> @@ -212,6 +215,7 @@ static void register_all(void)
>>       REGISTER_DECODER(H264_MEDIACODEC,   h264_mediacodec);
>>       REGISTER_DECODER(H264_MMAL,         h264_mmal);
>>       REGISTER_DECODER(H264_QSV,          h264_qsv);
>> +    REGISTER_DECODER(H264_RKMPP,        h264_rkmpp);
>>       REGISTER_DECODER(H264_VDA,          h264_vda);
>>   #if FF_API_VDPAU
>>       REGISTER_DECODER(H264_VDPAU,        h264_vdpau);
>> @@ -219,6 +223,7 @@ static void register_all(void)
>>       REGISTER_ENCDEC (HAP,               hap);
>>       REGISTER_DECODER(HEVC,              hevc);
>>       REGISTER_DECODER(HEVC_QSV,          hevc_qsv);
>> +    REGISTER_DECODER(HEVC_RKMPP,        hevc_rkmpp);
>>       REGISTER_DECODER(HNM4_VIDEO,        hnm4_video);
>>       REGISTER_DECODER(HQ_HQA,            hq_hqa);
>>       REGISTER_DECODER(HQX,               hqx);
>> @@ -372,6 +377,7 @@ static void register_all(void)
>>       REGISTER_DECODER(VP6F,              vp6f);
>>       REGISTER_DECODER(VP7,               vp7);
>>       REGISTER_DECODER(VP8,               vp8);
>> +    REGISTER_DECODER(VP8_RKMPP,         vp8_rkmpp);
>>       REGISTER_DECODER(VP9,               vp9);
>>       REGISTER_DECODER(VQA,               vqa);
>>       REGISTER_DECODER(BITPACKED,         bitpacked);
>> diff --git a/libavcodec/rkmppdec.c b/libavcodec/rkmppdec.c
>> new file mode 100644
>> index 0000000..eaffd12
>> --- /dev/null
>> +++ b/libavcodec/rkmppdec.c
>> @@ -0,0 +1,537 @@
>> +/*
>> + * RockChip MPP Video Decoder
>> + * Copyright (c) 2017 Lionel CHAZALLON
>> + *
>> + * 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 <drm/drm_fourcc.h>
> libdrm is already pkgconfiged, so you don't need the path.  (Also this path is wrong for recent versions.)
Changed.
>> +#include <pthread.h>
>> +#include <rockchip/mpp_buffer.h>
>> +#include <rockchip/rk_mpi.h>
>> +#include <time.h>
>> +#include <unistd.h>
>> +
>> +#include "avcodec.h"
>> +#include "decode.h"
>> +#include "internal.h"
>> +#include "libavutil/buffer.h"
>> +#include "libavutil/common.h"
>> +#include "libavutil/frame.h"
>> +#include "libavutil/hwcontext_drm.h"
>> +#include "libavutil/imgutils.h"
>> +#include "libavutil/log.h"
>> +
>> +#define RECEIVE_FRAME_TIMEOUT   100
>> +#define FRAMEGROUP_MAX_FRAMES   16
>> +
>> +typedef struct {
>> +    MppCtx ctx;
>> +    MppApi *mpi;
>> +    MppBufferGroup frame_group;
>> +
>> +    char first_frame;
>> +    char first_packet;
>> +    char eos_reached;
>> +} RKMPPDecoder;
>> +
>> +typedef struct {
>> +    AVClass *av_class;
>> +    AVBufferRef *decoder_ref;
>> +} RKMPPDecodeContext;
>> +
>> +typedef struct {
>> +    MppFrame frame;
>> +    AVBufferRef *decoder_ref;
>> +} RKMPPFrameContext;
>> +
>> +static MppCodingType ffrkmpp_get_codingtype(AVCodecContext *avctx)
> The ff prefix on symbols is special (symbols shared between files internal to a library), and should not be used here.  I think rename all of the functions here to just "rkmpp_*".
>
>> +{
>> +    switch (avctx->codec_id) {
>> +    case AV_CODEC_ID_H264:  return MPP_VIDEO_CodingAVC;
>> +    case AV_CODEC_ID_HEVC:  return MPP_VIDEO_CodingHEVC;
>> +    case AV_CODEC_ID_VP8:   return MPP_VIDEO_CodingVP8;
>> +    default:                return MPP_VIDEO_CodingUnused;
>> +    }
>> +}
>> +
>> +static int ffrkmpp_get_frameformat(MppFrameFormat mppformat)
>> +{
>> +    switch (mppformat) {
>> +    case MPP_FMT_YUV420SP:          return DRM_FORMAT_NV12;
>> +#ifdef DRM_FORMAT_NV12_10
>> +    case MPP_FMT_YUV420SP_10BIT:    return DRM_FORMAT_NV12_10;
>> +#endif
>> +    default:                        return 0;
>> +    }
>> +}
>> +
>> +static int ffrkmpp_write_data(AVCodecContext *avctx, uint8_t *buffer, int size, int64_t pts)
>> +{
>> +    RKMPPDecodeContext *rk_context = avctx->priv_data;
>> +    RKMPPDecoder *decoder = (RKMPPDecoder *)rk_context->decoder_ref->data;
>> +    MPP_RET ret = MPP_NOK;
>> +    MppPacket packet;
>> +
>> +    // create the MPP packet
>> +    ret = mpp_packet_init(&packet, buffer, size);
>> +    if (ret != MPP_OK) {
>> +        av_log(avctx, AV_LOG_ERROR, "Failed to init MPP packet (code = %d)\n", ret);
>> +        return AVERROR_UNKNOWN;
>> +    }
>> +
>> +    mpp_packet_set_pts(packet, pts);
>> +
>> +    if (!buffer)
>> +        mpp_packet_set_eos(packet);
>> +
>> +    ret = decoder->mpi->decode_put_packet(decoder->ctx, packet);
>> +    if (ret != MPP_OK) {
>> +        if (ret == MPP_ERR_BUFFER_FULL) {
>> +            av_log(avctx, AV_LOG_DEBUG, "Buffer full writing %d bytes to decoder\n", size);
>> +            ret = AVERROR(EAGAIN);
> MPP_RET is an enum, and need not be able to hold the range of AVERROR types - use a separate int.
>
> (Also in many other places through this patch.)
Changed all MPP_RET to ints.
>
>> +        } else
>> +            ret = AVERROR_UNKNOWN;
>> +    }
>> +    else
>> +        av_log(avctx, AV_LOG_DEBUG, "Wrote %d bytes to decoder\n", size);
>> +
>> +    mpp_packet_deinit(&packet);
>> +
>> +    return ret;
>> +}
>> +
>> +static int ffrkmpp_close_decoder(AVCodecContext *avctx)
>> +{
>> +    RKMPPDecodeContext *rk_context = avctx->priv_data;
>> +    av_buffer_unref(&rk_context->decoder_ref);
>> +    return 0;
>> +}
>> +
>> +static void ffrkmpp_release_decoder(void *opaque, uint8_t *data)
>> +{
>> +    RKMPPDecoder *decoder = (RKMPPDecoder *)data;
>> +
>> +    decoder->mpi->reset(decoder->ctx);
>> +    mpp_destroy(decoder->ctx);
>> +    decoder->ctx = NULL;
>> +
>> +    if (decoder->frame_group) {
>> +        mpp_buffer_group_put(decoder->frame_group);
>> +        decoder->frame_group = NULL;
>> +    }
>> +
>> +    av_free(decoder);
>> +}
>> +
>> +static int ffrkmpp_init_decoder(AVCodecContext *avctx)
>> +{
>> +    RKMPPDecodeContext *rk_context = avctx->priv_data;
>> +    RKMPPDecoder *decoder = NULL;
>> +    MppCodingType codectype = MPP_VIDEO_CodingUnused;
>> +    MPP_RET ret = MPP_NOK;
>> +    RK_S64 paramS64;
>> +    RK_S32 paramS32;
>> +
>> +    if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0)
>> +        return ret;
> This call seems to break it in ffmpeg because of silly hwaccel handling.  Since there is only one possible pixel format, you can just remove it (seems to get around this problem for me).
>
I will remove that, although I think that Kodi required that, we will 
investigate that case later.
>> +
>> +    avctx->pix_fmt = ret;
>> +
>> +    // create a decoder and a ref to it
>> +    decoder = av_mallocz(sizeof(RKMPPDecoder));
>> +    if (!decoder) {
>> +        ret = AVERROR(ENOMEM);
>> +        goto fail;
>> +    }
>> +
>> +    rk_context->decoder_ref = av_buffer_create((uint8_t *)decoder, sizeof(*decoder), ffrkmpp_release_decoder,
>> +                                               NULL, AV_BUFFER_FLAG_READONLY);
>> +    if (!rk_context->decoder_ref) {
>> +        av_free(decoder);
>> +        ret = AVERROR(ENOMEM);
>> +        goto fail;
>> +    }
>> +
>> +    av_log(avctx, AV_LOG_DEBUG, "Initializing RKMPP decoder.\n");
>> +
>> +    codectype = ffrkmpp_get_codingtype(avctx);
>> +    if (codectype == MPP_VIDEO_CodingUnused) {
>> +        av_log(avctx, AV_LOG_ERROR, "Unknown codec type (%d).\n", avctx->codec_id);
>> +        ret = AVERROR_UNKNOWN;
>> +        goto fail;
>> +    }
>> +
>> +    // Create the MPP context
>> +    ret = mpp_create(&decoder->ctx, &decoder->mpi);
>> +    if (ret != MPP_OK) {
>> +        av_log(avctx, AV_LOG_ERROR, "Failed to create MPP context (code = %d).\n", ret);
>> +        ret = AVERROR_UNKNOWN;
>> +        goto fail;
>> +    }
>> +
>> +    // initialize mpp
>> +    ret = mpp_init(decoder->ctx, MPP_CTX_DEC, codectype);
>> +    if (ret != MPP_OK) {
>> +        av_log(avctx, AV_LOG_ERROR, "Failed to initialize MPP context (code = %d).\n", ret);
>> +        ret = AVERROR_UNKNOWN;
>> +        goto fail;
>> +    }
>> +
>> +    // make decode calls blocking with a timeout
>> +    paramS32 = MPP_POLL_BLOCK;
>> +    ret = decoder->mpi->control(decoder->ctx, MPP_SET_OUTPUT_BLOCK, &paramS32);
>> +    if (ret != MPP_OK) {
>> +        av_log(avctx, AV_LOG_ERROR, "Failed to set blocking mode on MPI (code = %d).\n", ret);
>> +        ret = AVERROR_UNKNOWN;
>> +        goto fail;
>> +    }
>> +
>> +    paramS64 = RECEIVE_FRAME_TIMEOUT;
>> +    ret = decoder->mpi->control(decoder->ctx, MPP_SET_OUTPUT_BLOCK_TIMEOUT, &paramS64);
>> +    if (ret != MPP_OK) {
>> +        av_log(avctx, AV_LOG_ERROR, "Failed to set block timeout on MPI (code = %d).\n", ret);
>> +        ret = AVERROR_UNKNOWN;
>> +        goto fail;
>> +    }
>> +
>> +    ret = mpp_buffer_group_get_internal(&decoder->frame_group, MPP_BUFFER_TYPE_ION);
> ION seems odd to mention here.  Does this parameter actually do anything?  (MPP_BUFFER_TYPE_DRM would seem more appropriate.)

MPP_BUFFER_TYPE_ION & MPP_BUFFER_TYPE_DRM are said to do the same, but changed to MPP_BUFFER_TYPE_DRM for more clarity.

>> +    if (ret) {
>> +       av_log(avctx, AV_LOG_ERROR, "Failed to retrieve buffer group (code = %d)\n", ret);
>> +       ret = AVERROR_UNKNOWN;
>> +       goto fail;
>> +    }
>> +
>> +    ret = decoder->mpi->control(decoder->ctx, MPP_DEC_SET_EXT_BUF_GROUP, decoder->frame_group);
>> +    if (ret) {
>> +        av_log(avctx, AV_LOG_ERROR, "Failed to assign buffer group (code = %d)\n", ret);
>> +        ret = AVERROR_UNKNOWN;
>> +        goto fail;
>> +    }
>> +
>> +    ret = mpp_buffer_group_limit_config(decoder->frame_group, 0, FRAMEGROUP_MAX_FRAMES);
> FRAMEGROUP_MAX_FRAMES is 16 - is this sufficient to handle worst-case H.264/5, which can have 16 reference frames?
>
that is why it has been set to 16, if we have such a sample i can 
further test.
>> +    if (ret) {
>> +        av_log(avctx, AV_LOG_ERROR, "Failed to set buffer group limit (code = %d)\n", ret);
>> +        ret = AVERROR_UNKNOWN;
>> +        goto fail;
>> +    }
>> +
>> +    decoder->first_packet = 1;
>> +
>> +    av_log(avctx, AV_LOG_DEBUG, "RKMPP decoder initialized successfully.\n");
>> +    return 0;
>> +
>> +fail:
>> +    av_log(avctx, AV_LOG_ERROR, "Failed to initialize RKMPP decoder.\n");
>> +    ffrkmpp_close_decoder(avctx);
>> +    return ret;
>> +}
>> +
>> +static int ffrkmpp_send_packet(AVCodecContext *avctx, const AVPacket *avpkt)
>> +{
>> +    RKMPPDecodeContext *rk_context = avctx->priv_data;
>> +    RKMPPDecoder *decoder = (RKMPPDecoder *)rk_context->decoder_ref->data;
>> +    MPP_RET ret = MPP_NOK;
>> +
>> +    // handle EOF
>> +    if (!avpkt->size) {
>> +        av_log(avctx, AV_LOG_DEBUG, "End of stream.\n");
>> +        decoder->eos_reached = 1;
>> +        ret = ffrkmpp_write_data(avctx, NULL, 0, 0);
>> +        if (ret)
>> +            av_log(avctx, AV_LOG_ERROR, "Failed to send EOS to decoder (code = %d)\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    // on first packet, send extradata
>> +    if (decoder->first_packet) {
>> +        if (avctx->extradata_size) {
>> +            ret = ffrkmpp_write_data(avctx, avctx->extradata,
>> +                                            avctx->extradata_size,
>> +                                            avpkt->pts);
>> +            if (ret) {
>> +                av_log(avctx, AV_LOG_ERROR, "Failed to write extradata to decoder (code = %d)\n", ret);
>> +                return ret;
>> +            }
>> +        }
>> +        decoder->first_packet = 0;
>> +    }
>> +
>> +    // now send packet
>> +    ret = ffrkmpp_write_data(avctx, avpkt->data, avpkt->size, avpkt->pts);
>> +    if (ret)
>> +        av_log(avctx, AV_LOG_ERROR, "Failed to write data to decoder (code = %d)\n", ret);
> EAGAIN seems to be allowed here - is it actually ok, so it shouldn't be printing an error in that case?
added the check for EAGAIN which is indeed valid so that we dont display 
log when it's a normal thing.
>> +
>> +    return ret;
>> +}
>> +
>> +static void ffrkmpp_release_frame(void *opaque, uint8_t *data)
>> +{
>> +    AVDRMFrameDescriptor *desc = (AVDRMFrameDescriptor *)data;
>> +    AVBufferRef *framecontextref = (AVBufferRef *)opaque;
>> +    RKMPPFrameContext *framecontext = (RKMPPFrameContext *)framecontextref->data;
>> +
>> +    mpp_frame_deinit(&framecontext->frame);
>> +    av_buffer_unref(&framecontext->decoder_ref);
>> +    av_buffer_unref(&framecontextref);
>> +
>> +    av_free(desc);
>> +}
>> +
>> +static int ffrkmpp_retrieve_frame(AVCodecContext *avctx, AVFrame *frame)
>> +{
>> +    RKMPPDecodeContext *rk_context = avctx->priv_data;
>> +    RKMPPDecoder *decoder = (RKMPPDecoder *)rk_context->decoder_ref->data;
>> +    RKMPPFrameContext *framecontext = NULL;
>> +    AVBufferRef *framecontextref = NULL;
>> +    MPP_RET ret = MPP_NOK;
>> +    MppFrame mppframe = NULL;
>> +    MppBuffer buffer = NULL;
>> +    AVDRMFrameDescriptor *desc = NULL;
>> +    AVDRMLayerDescriptor *layer = NULL;
>> +    int retrycount = 0;
>> +
>> +    // on start of decoding, MPP can return -1, which is supposed to be expected
>> +    // this is due to some internal MPP init which is not completed, that will
>> +    // only happen in the first few frames queries, but should not be interpreted
>> +    // as an error, Therefore we need to retry a couple times when we get -1
>> +    // in order to let it time to complete it's init, then we sleep a bit between retries.
>> +retry_get_frame:
>> +    ret = decoder->mpi->decode_get_frame(decoder->ctx, &mppframe);
>> +    if (ret != MPP_OK && ret != MPP_ERR_TIMEOUT) {
>> +        if (retrycount < 5) {
>> +            av_log(avctx, AV_LOG_DEBUG, "Failed to get a frame, retrying (code = %d, retrycount = %d)\n", ret, retrycount);
>> +            usleep(10000);
>> +            retrycount++;
>> +            goto retry_get_frame;
>> +        } else {
>> +            av_log(avctx, AV_LOG_ERROR, "Failed to get a frame from MPP (code = %d)\n", ret);
>> +            goto fail;
>> +        }
>> +    }
> If this only happens at the beginning and is an error in other cases, please check that you are at the beginning of the stream and only do the retrying then.
>
yeah i added a further check so that this retries only until the first 
frame has been decoded.

>> +
>> +    if (mppframe) {
>> +        // Check wether we have a special frame or not
>> +        if (mpp_frame_get_info_change(mppframe)) {
>> +            av_log(avctx, AV_LOG_INFO, "Decoder noticed an info change (%dx%d), format=%d\n",
>> +                                        (int)mpp_frame_get_width(mppframe), (int)mpp_frame_get_height(mppframe),
>> +                                        (int)mpp_frame_get_fmt(mppframe));
>> +
>> +            avctx->width = mpp_frame_get_width(mppframe);
>> +            avctx->height = mpp_frame_get_height(mppframe);
>> +
>> +            decoder->mpi->control(decoder->ctx, MPP_DEC_SET_INFO_CHANGE_READY, NULL);
>> +            decoder->first_frame = 1;
>> +
>> +            // here decoder is fully initialized, we need to feed it again with data
>> +            ret = AVERROR(EAGAIN);
>> +            goto fail;
>> +        } else if (mpp_frame_get_eos(mppframe)) {
>> +            av_log(avctx, AV_LOG_DEBUG, "Received a EOS frame.\n");
>> +            decoder->eos_reached = 1;
>> +            ret = AVERROR_EOF;
>> +            goto fail;
>> +        } else if (mpp_frame_get_discard(mppframe)) {
>> +            av_log(avctx, AV_LOG_DEBUG, "Received a discard frame.\n");
>> +            ret = AVERROR(EAGAIN);
>> +            goto fail;
>> +        } else if (mpp_frame_get_errinfo(mppframe)) {
>> +            av_log(avctx, AV_LOG_DEBUG, "Received a errinfo frame.\n");
>> +            ret = AVERROR_EXIT;
> Maybe AVERROR_UNKNOWN.  Can you retrieve any more information about what the error actually is, since this case is currently silently falling over.
I changed the return code to AVERROR_UNKNOWN and the log message to error.
I didn't find any futher way to get additionnal informt, they don't seem 
to be exported by the API.
>
>> +            goto fail;
>> +        }
> Would the EAGAIN cases here be better going around to call decode_get_frame() again?  (To make sure it satisfies the EAGAIN semantics of receive_frame.)
>
>> +
>> +        // here we should have a valid frame
>> +        av_log(avctx, AV_LOG_DEBUG, "Received a frame.\n");
>> +
>> +        // setup general frame fields
>> +        frame->format   = AV_PIX_FMT_DRM_PRIME;
>> +        frame->width    = mpp_frame_get_width(mppframe);
>> +        frame->height   = mpp_frame_get_height(mppframe);
>> +        frame->pts      = mpp_frame_get_pts(mppframe);
>> +
>> +        // now setup the frame buffer info
>> +        buffer = mpp_frame_get_buffer(mppframe);
>> +        if (buffer) {
>> +            desc = av_mallocz(sizeof(AVDRMFrameDescriptor));
>> +            if (!desc) {
>> +                ret = AVERROR(ENOMEM);
>> +                goto fail;
>> +            }
>> +
>> +            desc->nb_objects = 1;
>> +            desc->objects[0].fd = mpp_buffer_get_fd(buffer);
>> +            desc->objects[0].size = sizeof(AVDRMObjectDescriptor);
>> +
>> +            desc->nb_layers = 1;
>> +            layer = &desc->layers[0];
>> +            layer->format = ffrkmpp_get_frameformat(mpp_frame_get_fmt(mppframe));
>> +            layer->nb_planes = 2;
>> +
>> +            layer->planes[0].object_index = 0;
>> +            layer->planes[0].offset = 0;
>> +            layer->planes[0].pitch = mpp_frame_get_hor_stride(mppframe);
>> +
>> +            layer->planes[1].object_index = 0;
>> +            layer->planes[1].offset = layer->planes[0].pitch * mpp_frame_get_ver_stride(mppframe);
>> +            layer->planes[1].pitch = layer->planes[0].pitch;
>> +
>> +            // we also allocate a struct in buf[0] that will allow to hold additionnal information
>> +            // for releasing properly MPP frames and decoder
>> +            framecontextref = av_buffer_allocz(sizeof(*framecontext));
>> +            if (!framecontextref) {
>> +                ret = AVERROR(ENOMEM);
>> +                goto fail;
>> +            }
>> +
>> +            // MPP decoder needs to be closed only when all frames have been released.
>> +            framecontext = (RKMPPFrameContext *)framecontextref->data;
>> +            framecontext->decoder_ref = av_buffer_ref(rk_context->decoder_ref);
>> +            framecontext->frame = mppframe;
>> +
>> +            frame->data[0]  = (uint8_t *)desc;
>> +            frame->buf[0]   = av_buffer_create((uint8_t *)desc, sizeof(*desc), ffrkmpp_release_frame,
>> +                                               framecontextref, AV_BUFFER_FLAG_READONLY);
>> +
>> +            if (!frame->buf[0]) {
>> +                ret = AVERROR(ENOMEM);
>> +                goto fail;
>> +            }
> This frame needs an hw_frames_ctx to be usable with hwcontext.  It doesn't actually need to do anything, but does need to be the DRM type and have sensible metadata.  I think make it on the info change (when the new stream dimensions / format are known), then tag every output frame with it here.
>
Will add the hw_frames_ctx stuff in next patch version.
>> +
>> +            decoder->first_frame = 0;
>> +            return 0;
>> +        } else {
>> +            av_log(avctx, AV_LOG_ERROR, "Failed to retrieve the frame buffer, frame is dropped (code = %d)\n", ret);
>> +            mpp_frame_deinit(&mppframe);
>> +        }
>> +    } else if (decoder->eos_reached) {
>> +        return AVERROR_EOF;
>> +    } else if (ret == MPP_ERR_TIMEOUT) {
>> +        av_log(avctx, AV_LOG_DEBUG, "Timeout when trying to get a frame from MPP\n");
>> +    }
>> +
>> +    return AVERROR(EAGAIN);
>> +
>> +fail:
>> +    if (mppframe)
>> +        mpp_frame_deinit(&mppframe);
>> +
>> +    if (framecontext)
>> +        av_buffer_unref(&framecontext->decoder_ref);
>> +
>> +    if (framecontextref)
>> +        av_buffer_unref(&framecontextref);
>> +
>> +    if (desc)
>> +        av_free(desc);
>> +
>> +    return ret;
>> +}
>> +
>> +static int ffrkmpp_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>> +{
>> +    RKMPPDecodeContext *rk_context = avctx->priv_data;
>> +    RKMPPDecoder *decoder = (RKMPPDecoder *)rk_context->decoder_ref->data;
>> +    MPP_RET ret = MPP_NOK;
>> +    AVPacket pkt = {0};
>> +    RK_S32 freeslots;
>> +
>> +    if (!decoder->eos_reached) {
>> +        // we get the available slots in decoder
>> +        ret = decoder->mpi->control(decoder->ctx, MPP_DEC_GET_FREE_PACKET_SLOT_COUNT, &freeslots);
>> +        if (ret != MPP_OK) {
>> +            av_log(avctx, AV_LOG_ERROR, "Failed to get decoder free slots (code = %d).\n", ret);
>> +            return ret;
>> +        }
>> +
>> +        if (freeslots > 0) {
>> +            ret = ff_decode_get_packet(avctx, &pkt);
>> +            if (ret < 0 && ret != AVERROR_EOF) {
>> +                return ret;
>> +            }
>> +
>> +            ret = ffrkmpp_send_packet(avctx, &pkt);
>> +            av_packet_unref(&pkt);
>> +
>> +            if (ret < 0) {
>> +                av_log(avctx, AV_LOG_ERROR, "Failed to send packet to decoder (code = %d)\n", ret);
>> +                return ret;
>> +            }
>> +        }
>> +
>> +        // make sure we keep decoder full
>> +        if (freeslots > 1 && decoder->first_frame)
>> +            return AVERROR(EAGAIN);
>> +    }
>> +
>> +    return ffrkmpp_retrieve_frame(avctx, frame);
>> +}
>> +
>> +static void ffrkmpp_flush(AVCodecContext *avctx)
>> +{
>> +    RKMPPDecodeContext *rk_context = avctx->priv_data;
>> +    RKMPPDecoder *decoder = (RKMPPDecoder *)rk_context->decoder_ref->data;
>> +    MPP_RET ret = MPP_NOK;
>> +
>> +    av_log(avctx, AV_LOG_DEBUG, "Flush.\n");
>> +
>> +    ret = decoder->mpi->reset(decoder->ctx);
>> +    if (ret == MPP_OK) {
>> +        decoder->first_frame = 1;
>> +        decoder->first_packet = 1;
>> +    } else
>> +        av_log(avctx, AV_LOG_ERROR, "Failed to reset MPI (code = %d)\n", ret);
>> +}
>> +
>> +#define FFRKMPP_DEC_HWACCEL(NAME, ID) \
>> +  AVHWAccel ff_##NAME##_rkmpp_hwaccel = { \
>> +      .name     = #NAME "_rkmpp", \
>> +      .type     = AVMEDIA_TYPE_VIDEO,\
>> +      .id       = ID, \
>> +      .pix_fmt  = AV_PIX_FMT_DRM_PRIME,\
>> +  };
> As noted above, the hwaccel doesn't do anything.
For that part, when i tried to remove it, i got some linker error that 
was waiting for this to be defined.

>> +
>> +#define FFRKMPP_DEC_CLASS(NAME) \
>> +    static const AVClass ffrkmpp_##NAME##_dec_class = { \
>> +        .class_name = "rkmpp_" #NAME "_dec", \
>> +        .version    = LIBAVUTIL_VERSION_INT, \
>> +    };
>> +
>> +#define FFRKMPP_DEC(NAME, ID, BSFS) \
>> +    FFRKMPP_DEC_CLASS(NAME) \
>> +    FFRKMPP_DEC_HWACCEL(NAME, ID) \
>> +    AVCodec ff_##NAME##_rkmpp_decoder = { \
>> +        .name           = #NAME "_rkmpp", \
>> +        .long_name      = NULL_IF_CONFIG_SMALL(#NAME " (rkmpp)"), \
>> +        .type           = AVMEDIA_TYPE_VIDEO, \
>> +        .id             = ID, \
>> +        .priv_data_size = sizeof(RKMPPDecodeContext), \
>> +        .init           = ffrkmpp_init_decoder, \
>> +        .close          = ffrkmpp_close_decoder, \
>> +        .receive_frame  = ffrkmpp_receive_frame, \
>> +        .flush          = ffrkmpp_flush, \
>> +        .priv_class     = &ffrkmpp_##NAME##_dec_class, \
>> +        .capabilities   = AV_CODEC_CAP_DELAY, \
>> +        .caps_internal  = FF_CODEC_CAP_SETS_PKT_DTS, \
>> +        .pix_fmts       = (const enum AVPixelFormat[]) { AV_PIX_FMT_DRM_PRIME, \
>> +                                                         AV_PIX_FMT_NONE}, \
>> +        .bsfs           = #BSFS, \
>> +    };
>> +
>> +FFRKMPP_DEC(h264, AV_CODEC_ID_H264, h264_mp4toannexb)
>> +FFRKMPP_DEC(hevc, AV_CODEC_ID_HEVC, hevc_mp4toannexb)
>> +FFRKMPP_DEC(vp8,  AV_CODEC_ID_VP8,  )
>>
> Trying to decode H.264 doesn't work for me - I get repeated errinfo frames and then after a while it gets stuck entirely.  I tried a few random files and some of the conformance tests, and all failed in the same way.
As we discussed on IRC MPP needs to be built with the proper CMAKE 
defines which is why it was failing for you
>
> Decoding H.265 seems to work, though I need to do a bit more to check the result (hwdownload doesn't work because there is not hwframe context, as noted above).
hw_frame_context will land in netx patch
>
> VP8 wouldn't initialise at all: something missing, maybe - probably not a problem with this patch.
There was an issue with VP8 & BSF, will be fixed in next patch.
>
> Thanks,
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



More information about the ffmpeg-devel mailing list