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

[wm4]

On Tue, 5 Sep 2017 20:19:09 +0100
Mark Thompson <sw at jkqxz.net> wrote:

> On 05/09/17 19:55, LongChair . wrote:> Le 05/09/2017 à 19:19, Mark Thompson a écrit :
> >> On 05/09/17 12:44, LongChair . wrote:  
> >>> From: LongChair <LongChair at hotmail.com>
> >>>
> >>> 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 (TinkerBoard) and RK3328.
> >>> ---
> >>>   Changelog              |   1 +
> >>>   configure              |  15 +-
> >>>   libavcodec/Makefile    |   3 +
> >>>   libavcodec/allcodecs.c |   3 +
> >>>   libavcodec/rkmppdec.c  | 567 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>>   5 files changed, 588 insertions(+), 1 deletion(-)
> >>>   create mode 100644 libavcodec/rkmppdec.c
> >>>
> >>> ...
> >>> @@ -6008,7 +6016,12 @@ 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; }
> >>> -  
> 
> Oops, sorry, that wasn't very clear.  I meant leave this ^ blank line, and put your check above it.  The form of it in the patch is fine.
> 
> >>> +enabled rkmpp             && { { require_pkg_config rockchip_mpp rockchip/rk_mpi.h mpp_create ||
> >>> +                                 die "ERROR : Rockchip MPP was not found."; }  &&
> >>> +                               { check_func_headers rockchip/rk_mpi_cmd.h "MPP_DEC_GET_FREE_PACKET_SLOT_COUNT" ||
> >>> +                                 die "ERROR: Rockchip MPP is outdated, please get a more recent one."; } &&
> >>> +                               { enabled libdrm || die "ERROR: rkmpp requires --enable-libdrm"; }
> >>> +                             }  
> >> Keep the blank line after the list of normal checks and before the more complex cases below.  
> > i Hope something like this makes it more readable :
> > 
> > enabled rkmpp             && {
> > 
> >                                 { require_pkg_config rockchip_mpp rockchip/rk_mpi.h mpp_create ||
> > 
> >                                   die "ERROR : Rockchip MPP was not found."; }
> > 
> >                            &&   { check_func_headers rockchip/rk_mpi_cmd.h "MPP_DEC_GET_FREE_PACKET_SLOT_COUNT" ||
> > 
> >                                   die "ERROR: Rockchip MPP is outdated, please get a more recent one."; }
> > 
> >                            &&   { enabled libdrm ||
> > 
> >                                   die "ERROR: rkmpp requires --enable-libdrm"; }
> > 
> >                               }
> >   
> >> Also needs a rebase - qtkit_indev was removed.  
> > Will rebase  
> >>> ...
> >>> +
> >>> +        // 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);  
> >> Can you get other frame metadata here?  (SAR, all of the colour stuff?)  
> > well there are a few other functions that i didn't use that seem to be 
> > colorspace related here 
> > https://github.com/rockchip-linux/mpp/blob/30032ae303956788bd5b709c9bc70867b0ea95d6/inc/mpp_frame.h#L242-L253
> > I never looked into those yet, but if you feel like some would be 
> > helpful and important I could have another looks there.  
> >> Does anything sensible happen with interlaced streams?  
> > there are flags defined for this in the MPP headers, but i don't know if 
> > they are effectively used in MppFrame. Would need to check further.
> > https://github.com/rockchip-linux/mpp/blob/30032ae303956788bd5b709c9bc70867b0ea95d6/inc/mpp_frame.h#L28-L43  
> 
> Yes, exactly those - all five of the colour-related functions.  (I see the point made earlier that this file is oddly reminiscent of libavutil/pixfmt.h (including the comments), so you can probably just put the values directly into the AVFrame fields...)

Well, in both cases, they're probably just the bitstream values.