On Thu, Oct 27, 2016 at 11:58:50AM +0000, Ruta Gadkari wrote:
> --- a/configure
> +++ b/configure
> @@ -1223,6 +1224,7 @@ HWACCEL_LIBRARY_NONFREE_LIST="
> "
> HWACCEL_LIBRARY_LIST="
> $HWACCEL_LIBRARY_NONFREE_LIST
> + cuvid
Isn't cuvid nonfree?
> @@ -2125,8 +2128,10 @@ vda_deps="VideoDecodeAcceleration_VDADecoder_h
> pthreads"
> vda_extralibs="-framework CoreFoundation -framework VideoDecodeAcceleration
> -framework QuartzCore"
> vdpau_deps="vdpau_vdpau_h vdpau_vdpau_x11_h"
>
> +h263_cuvid_hwaccel_deps="cuda cuvid CUVIDMPEG4PICPARAMS"
Does this depend directly on cuda or only directly on cuvid and on cuda
only through a transitive dependency. If the latter is true, only cuvid
needs to be mentioned here.
same below
> @@ -2200,6 +2213,10 @@ vaapi_encode_deps="vaapi"
>
> +h263_cuvid_decoder_deps="cuda cuvid CUVIDMPEG4PICPARAMS"
> +h264_cuvid_decoder_deps="cuda cuvid CUVIDH264PICPARAMS"
same, more below
> +check_type "cuviddec.h" "CUVIDMPEG4PICPARAMS"
> +check_type "cuviddec.h" "CUVIDVC1PICPARAMS"
> +check_type "cuviddec.h" "CUVIDVP8PICPARAMS"
> +check_type "cuviddec.h" "CUVIDVP9PICPARAMS"
> +
> +
> if ! disabled w32threads && ! enabled pthreads; then
> check_func_headers "windows.h process.h" _beginthreadex &&
> enable w32threads || disable w32threads
> fi
>
> +# Enable CUVID by default if CUDA is enabled
> +if enabled cuda && ! disabled cuvid; then
> + enable cuvid
> +fi
> +
> # check for some common methods of building with pthread support
> # do this before the optional library checks as some of them require pthreads
> if ! disabled pthreads && ! enabled w32threads; then
Please do not add cuvid stuff into the middle of the (p)threads block.
> @@ -4608,6 +4657,10 @@ enabled avisynth && { check_lib
> "avisynth/avisynth_c.h windows.h" LoadL
> check_lib "avxsynth/avxsynth_c.h dlfcn.h"
> dlopen -ldl ||
> die "ERROR: LoadLibrary/dlopen not found, or
> avisynth header not found"; }
> enabled cuda && check_lib cuda.h cuInit -lcuda
> +enabled cuvid && { check_lib cuviddec.h cuvidCreateDecoder
> -lnvcuvid ||
> + die "ERROR: CUVID not found"; } &&
> + { enabled cuda ||
> + die "ERROR: CUVID requires CUDA"; }
No need to doublecheck for cuda and use require() instead of check_lib().
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -256,6 +256,7 @@ OBJS-$(CONFIG_H264_DECODER) += h264dec.o
> h264_cabac.o h264_cavlc.o \
> h264_mb.o h264_picture.o \
> h264_refs.o h264_sei.o \
> h264_slice.o h264data.o
> +OBJS-$(CONFIG_H264_CUVID_DECODER) += cuvid.o
> OBJS-$(CONFIG_H264_MMAL_DECODER) += mmaldec.o
> OBJS-$(CONFIG_H264_NVENC_ENCODER) += nvenc_h264.o
> OBJS-$(CONFIG_H264_OMX_ENCODER) += omx.o
> @@ -267,6 +268,7 @@ OBJS-$(CONFIG_HAP_ENCODER) += hapenc.o hap.o
> OBJS-$(CONFIG_HEVC_DECODER) += hevcdec.o hevc_mvs.o hevc_ps.o
> hevc_sei.o \
> hevc_cabac.o hevc_refs.o
> hevcpred.o \
> hevcdsp.o hevc_filter.o
> h2645_parse.o hevc_data.o
> +OBJS-$(CONFIG_HEVC_CUVID_DECODER) += cuvid.o
> OBJS-$(CONFIG_HEVC_NVENC_ENCODER) += nvenc_hevc.o
> OBJS-$(CONFIG_HEVC_QSV_DECODER) += qsvdec_h2645.o
> OBJS-$(CONFIG_HEVC_QSV_ENCODER) += qsvenc_hevc.o hevc_ps_enc.o
> h2645_parse.o
> @@ -460,6 +462,7 @@ OBJS-$(CONFIG_VC1_DECODER) += vc1dec.o
> vc1_block.o vc1_loopfilter.o
> vc1_mc.o vc1_pred.o vc1.o
> vc1data.o \
> msmpeg4dec.o msmpeg4.o
> msmpeg4data.o \
> wmv2data.o
> +OBJS-$(CONFIG_VC1_CUVID_DECODER) += cuvid.o
> OBJS-$(CONFIG_VC1_MMAL_DECODER) += mmaldec.o
> OBJS-$(CONFIG_VCR1_DECODER) += vcr1.o
> OBJS-$(CONFIG_VMDAUDIO_DECODER) += vmdaudio.o
> @@ -475,8 +478,10 @@ OBJS-$(CONFIG_VP6_DECODER) += vp6.o vp56.o
> vp56data.o \
> vp6dsp.o vp56rac.o
> OBJS-$(CONFIG_VP7_DECODER) += vp8.o vp56rac.o
> OBJS-$(CONFIG_VP8_DECODER) += vp8.o vp56rac.o
> +OBJS-$(CONFIG_VP8_CUVID_DECODER) += cuvid.o
> OBJS-$(CONFIG_VP9_DECODER) += vp9.o vp9data.o vp9dsp.o \
> vp9block.o vp9prob.o vp9mvs.o
> vp56rac.o
> +OBJS-$(CONFIG_VP9_CUVID_DECODER) += cuvid.o
> OBJS-$(CONFIG_VQA_DECODER) += vqavideo.o
> OBJS-$(CONFIG_WAVPACK_DECODER) += wavpack.o
> OBJS-$(CONFIG_WEBP_DECODER) += webp.o
This is missing many of the decoders and hwaccels that you added.
Please test that all added decoders and hwaccels compile standalone, see
https://www.libav.org/documentation/developer.html#New-codecs-or-formats-checklist
> --- /dev/null
> +++ b/libavcodec/cuvid.c
> @@ -0,0 +1,917 @@
> +
> +#include "libavutil/buffer.h"
> +#include "libavutil/mathematics.h"
> +#include "libavutil/hwcontext.h"
> +#include "libavutil/hwcontext_cuda.h"
> +#include "libavutil/fifo.h"
> +#include "libavutil/log.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/common.h"
> +
> +#include "avcodec.h"
> +#include "internal.h"
> +
> +#include <nvcuvid.h>
Move the nvcuvid.h system #include above all other #includes so that side
effects are avoided.
> +typedef struct CuvidContext
> +{
typedef struct CuvidContext {
> +typedef struct CuvidParsedFrame
> +{
same
> +static int check_cu(AVCodecContext *avctx, CUresult err, const char *func)
> +{
> + const char *err_name;
> + const char *err_string;
> +
> + av_log(avctx, AV_LOG_TRACE, "Calling %s\n", func);
All the trace output seems too silly to keep in production code to me.
Others might disagree. Same below...
> +static int CUDAAPI cuvid_handle_video_sequence(void *opaque, CUVIDEOFORMAT*
> format)
> +{
> + avctx->width = format->display_area.right;
> + avctx->height = format->display_area.bottom;
nit: align =
> + avctx->color_primaries =
> format->video_signal_description.color_primaries;
> + avctx->color_trc =
> format->video_signal_description.transfer_characteristics;
> + avctx->colorspace = format->video_signal_description.matrix_coefficients;
same
more below
> + if (ctx->cudecoder
> + && avctx->coded_width == format->coded_width
> + && avctx->coded_height == format->coded_height
> + && ctx->chroma_format == format->chroma_format
> + && ctx->codec_type == format->codec)
if (ctx->cudecoder &&
avctx->coded_width == format->coded_width &&
avctx->coded_height == format->coded_height &&
ctx->chroma_format == format->chroma_format &&
ctx->codec_type == format->codec)
> + if (hwframe_ctx->pool && (
> + hwframe_ctx->width < avctx->width ||
> + hwframe_ctx->height < avctx->height ||
> + hwframe_ctx->format != AV_PIX_FMT_CUDA ||
> + hwframe_ctx->sw_format != AV_PIX_FMT_NV12)) {
same
> + if (format->chroma_format != cudaVideoChromaFormat_420) {
> + av_log(avctx, AV_LOG_ERROR, "Chroma formats other than 420 are not
> supported\n");
> + ctx->internal_error = AVERROR(EINVAL);
Here and in other places: if the problem is an invalid bitstream rather
than invalid user input, the error code should be AVERROR_INVALIDDATA.
This is also a candidate for avpriv_report_missing_feature().
> + memset(&cuinfo, 0, sizeof(cuinfo));
You can avoid the memset through a zero initialization above at declaration.
> + if (ctx->deint_mode != cudaVideoDeinterlaceMode_Weave)
> + avctx->framerate = av_mul_q(avctx->framerate, (AVRational){2, 1});
nit: spaces inside {}
> +static int cuvid_decode_packet(AVCodecContext *avctx, const AVPacket *avpkt)
> +{
> + CUVIDSOURCEDATAPACKET cupkt;
> + AVPacket filter_packet = { 0 };
> + AVPacket filtered_packet = { 0 };
> +
> + memset(&cupkt, 0, sizeof(cupkt));
You even have other zero initializations here, use it for all variables.
> + // cuvidParseVideoData doesn't return an error just because stuff
> failed...
> + if (ctx->internal_error) {
> + av_log(avctx, AV_LOG_ERROR, "cuvid decode callback error\n");
> + ret = ctx->internal_error;
> + goto error;
> + }
> +
> +error:
This goto is rather silly.
> +static int cuvid_output_frame(AVCodecContext *avctx, AVFrame *frame)
> +{
> + if (av_fifo_size(ctx->frame_queue)) {
> + CuvidParsedFrame parsed_frame;
> + CUVIDPROCPARAMS params;
> +
> + memset(¶ms, 0, sizeof(params));
Another candidate for zero initialization..
> + tmp_frame->data[0] = (uint8_t*)mapped_frame;
> + tmp_frame->data[1] = (uint8_t*)(mapped_frame +
> avctx->coded_height * pitch);
(uint8_t *)
> + /* CUVIDs opaque reordering breaks the internal pkt logic.
> + * So set pkt_pts and clear all the other pkt_ fields.
> + */
> +#if FF_API_PKT_PTS
> +FF_DISABLE_DEPRECATION_WARNINGS
> + frame->pkt_pts = frame->pts;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> + //av_frame_set_pkt_pos(frame, -1);
> + //av_frame_set_pkt_duration(frame, 0);
> + //av_frame_set_pkt_size(frame, -1);
Please don't leave commented-out cruft behind.
> +error:
> + if (mapped_frame)
> + eret = CHECK_CU(cuvidUnmapVideoFrame(ctx->cudecoder, mapped_frame));
> +
> + eret = CHECK_CU(cuCtxPopCurrent(&dummy));
> +
> + if (eret < 0)
> + return eret;
You discard the first eret value, maybe you should check it or not bother
to set it if you intend to discard it?
> +static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS
> *cuparseinfo)
> +{
> + CUVIDDECODECREATEINFO cuinfo;
> +
> + memset(&cuinfo, 0, sizeof(cuinfo));
see above
> + cuinfo.CodecType = cuparseinfo->CodecType;
> + cuinfo.ChromaFormat = cudaVideoChromaFormat_420;
> + cuinfo.OutputFormat = cudaVideoSurfaceFormat_NV12;
> +
> + cuinfo.ulWidth = 1280;
> + cuinfo.ulHeight = 720;
> + cuinfo.ulTargetWidth = cuinfo.ulWidth;
> + cuinfo.ulTargetHeight = cuinfo.ulHeight;
> +
> + cuinfo.target_rect.left = 0;
> + cuinfo.target_rect.top = 0;
> + cuinfo.target_rect.right = cuinfo.ulWidth;
> + cuinfo.target_rect.bottom = cuinfo.ulHeight;
> +
> + cuinfo.ulNumDecodeSurfaces = MAX_FRAME_COUNT;
> + cuinfo.ulNumOutputSurfaces = 1;
> + cuinfo.ulCreationFlags = cudaVideoCreate_PreferCUVID;
> + cuinfo.bitDepthMinus8 = 0;
> +
> + cuinfo.DeinterlaceMode = cudaVideoDeinterlaceMode_Weave;
more opportunities for aligning =
> +static void cuvid_flush(AVCodecContext *avctx)
> +{
> + ctx->prev_pts = INT64_MIN;
> + ctx->decoder_flushing = 0;
> +
> + return;
> + error:
> + av_log(avctx, AV_LOG_ERROR, "CUDA reinit on flush failed\n");
> +}
IMO pointless return.
> +#define DEFINE_CUVID_CODEC(x, X) \
Calling a define DEFINE_ is pretty redundant, so don't call it define,
we already know it's a define and therefore calling it refine would be
redundant. ;)
> + static const AVClass x##_cuvid_class = { \
> + .id = AV_CODEC_ID_##X, \
> + .id = AV_CODEC_ID_##X, \
> + .priv_class = &x##_cuvid_class, \
spaces around ##
Diego
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
