| Message ID | 1485019650-30152-1-git-send-email-pkoshevoy@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Sat, Jan 21, 2017 at 10:27 AM, <pkoshevoy@gmail.com> wrote: > From: Pavel Koshevoy <pkoshevoy@gmail.com> > > Evidently CUVID doesn't support decoding 422 or 444 chroma formats, > and only a limited set of resolutions per codec are supported. > > Unfortunately CUVID silently drops packets for video of unsupported > resolution. However, it will error-out at cuvidCreateDecoder call > if the indicated video resolution is not supported. > > Given that stream resolution and pixel format are typically known as > a result of probing it is better to use this information during > avcodec_open2 call to fail immediately, rather than proceeding to > decode and never receiving any frames from the decoder nor receiving > any indication of decode failure. > --- > libavcodec/cuvid.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 57 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c > index 8fc713d..f9c29a1 100644 > --- a/libavcodec/cuvid.c > +++ b/libavcodec/cuvid.c > @@ -612,7 +612,11 @@ static av_cold int cuvid_decode_end(AVCodecContext *avctx) > return 0; > } > > -static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cuparseinfo) > +static int cuvid_test_dummy_decoder(AVCodecContext *avctx, > + const CUVIDPARSERPARAMS *cuparseinfo, > + cudaVideoChromaFormat probed_chroma_format, > + int probed_width, > + int probed_height) > { > CuvidContext *ctx = avctx->priv_data; > CUVIDDECODECREATEINFO cuinfo; > @@ -622,11 +626,11 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu > memset(&cuinfo, 0, sizeof(cuinfo)); > > cuinfo.CodecType = cuparseinfo->CodecType; > - cuinfo.ChromaFormat = cudaVideoChromaFormat_420; > + cuinfo.ChromaFormat = probed_chroma_format; > cuinfo.OutputFormat = cudaVideoSurfaceFormat_NV12; > > - cuinfo.ulWidth = 1280; > - cuinfo.ulHeight = 720; > + cuinfo.ulWidth = probed_width; > + cuinfo.ulHeight = probed_height; > cuinfo.ulTargetWidth = cuinfo.ulWidth; > cuinfo.ulTargetHeight = cuinfo.ulHeight; > > @@ -653,6 +657,36 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu > return 0; > } > > +static int convert_to_cuda_video_chroma_format(enum AVPixelFormat pix_fmt, > + cudaVideoChromaFormat *out) > +{ > + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt); > + if (!(out && desc && > + (desc->nb_components == 1 || desc->nb_components == 3) && > + (desc->log2_chroma_w < 2 && desc->log2_chroma_h < 2))) > + { > + return AVERROR(EINVAL); > + } > + > + if (desc->nb_components == 1) > + { > + *out = cudaVideoChromaFormat_Monochrome; > + } > + else if (desc->flags == AV_PIX_FMT_FLAG_PLANAR) > + { > + *out = ((desc->log2_chroma_w == 0) ? cudaVideoChromaFormat_444 : > + (desc->log2_chroma_h == 0) ? cudaVideoChromaFormat_422 : > + cudaVideoChromaFormat_420); > + } > + else > + { > + return AVERROR(EINVAL); > + } > + > + // unfortunately, 420 is the only one that works: > + return (*out == cudaVideoChromaFormat_420) ? 0 : AVERROR_EXTERNAL; > +} > + > static av_cold int cuvid_decode_init(AVCodecContext *avctx) > { > CuvidContext *ctx = avctx->priv_data; > @@ -663,12 +697,27 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) > CUcontext cuda_ctx = NULL; > CUcontext dummy; > const AVBitStreamFilter *bsf; > + cudaVideoChromaFormat probed_chroma_format; > + int probed_width; > + int probed_height; > int ret = 0; > > enum AVPixelFormat pix_fmts[3] = { AV_PIX_FMT_CUDA, > AV_PIX_FMT_NV12, > AV_PIX_FMT_NONE }; > > + enum AVPixelFormat probed_pix_fmt = (avctx->pix_fmt <= 0 ? > + AV_PIX_FMT_YUV420P : > + avctx->pix_fmt); > + > + ret = convert_to_cuda_video_chroma_format(probed_pix_fmt, &probed_chroma_format); > + if (ret < 0) { > + // pixel format is not supported: > + return ret; > + } > + probed_width = avctx->coded_width ? avctx->coded_width : 1280; > + probed_height = avctx->coded_height ? avctx->coded_height : 720; > + > // Accelerated transcoding scenarios with 'ffmpeg' require that the > // pix_fmt be set to AV_PIX_FMT_CUDA early. The sw_pix_fmt, and the > // pix_fmt for non-accelerated transcoding, do not need to be correct > @@ -824,7 +873,10 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) > if (ret < 0) > goto error; > > - ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo); > + ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo, > + probed_chroma_format, > + probed_width, > + probed_height); > if (ret < 0) > goto error; > > -- > 2.9.2 > Despite wm4 objections I am resubmitting this patch, because I can find no other reasonable method for rejecting ffmpeg cuvid decoder when it can't handle a given video stream. An unreasonable alternative would be to duplicate the work that cuvid.c does internally on my application side, and that would be of little benefit to me and to ffmpeg -- I might as well use the nvenc APIs directly then. I still see this patch as an improvement. Pavel.
On Sat, Jan 21, 2017 at 10:35 AM, Pavel Koshevoy <pkoshevoy@gmail.com> wrote: <snip> > Despite wm4 objections I am resubmitting this patch, because I can > find no other reasonable method for rejecting ffmpeg cuvid decoder > when it can't handle a given video stream. An unreasonable > alternative would be to duplicate the work that cuvid.c does > internally on my application side, and that would be of little benefit > to me and to ffmpeg -- I might as well use the nvenc APIs directly I meant cuvid, not nvenc of course ... I get those mixed up sometimes, sorry about that > then. I still see this patch as an improvement. > > Pavel.
On Sat, 21 Jan 2017 10:35:50 -0700 Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > On Sat, Jan 21, 2017 at 10:27 AM, <pkoshevoy@gmail.com> wrote: > > From: Pavel Koshevoy <pkoshevoy@gmail.com> > > > > Evidently CUVID doesn't support decoding 422 or 444 chroma formats, > > and only a limited set of resolutions per codec are supported. > > > > Unfortunately CUVID silently drops packets for video of unsupported > > resolution. However, it will error-out at cuvidCreateDecoder call > > if the indicated video resolution is not supported. > > > > Given that stream resolution and pixel format are typically known as > > a result of probing it is better to use this information during > > avcodec_open2 call to fail immediately, rather than proceeding to > > decode and never receiving any frames from the decoder nor receiving > > any indication of decode failure. > > --- > > libavcodec/cuvid.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 57 insertions(+), 5 deletions(-) > > > > diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c > > index 8fc713d..f9c29a1 100644 > > --- a/libavcodec/cuvid.c > > +++ b/libavcodec/cuvid.c > > @@ -612,7 +612,11 @@ static av_cold int cuvid_decode_end(AVCodecContext *avctx) > > return 0; > > } > > > > -static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cuparseinfo) > > +static int cuvid_test_dummy_decoder(AVCodecContext *avctx, > > + const CUVIDPARSERPARAMS *cuparseinfo, > > + cudaVideoChromaFormat probed_chroma_format, > > + int probed_width, > > + int probed_height) > > { > > CuvidContext *ctx = avctx->priv_data; > > CUVIDDECODECREATEINFO cuinfo; > > @@ -622,11 +626,11 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu > > memset(&cuinfo, 0, sizeof(cuinfo)); > > > > cuinfo.CodecType = cuparseinfo->CodecType; > > - cuinfo.ChromaFormat = cudaVideoChromaFormat_420; > > + cuinfo.ChromaFormat = probed_chroma_format; > > cuinfo.OutputFormat = cudaVideoSurfaceFormat_NV12; > > > > - cuinfo.ulWidth = 1280; > > - cuinfo.ulHeight = 720; > > + cuinfo.ulWidth = probed_width; > > + cuinfo.ulHeight = probed_height; > > cuinfo.ulTargetWidth = cuinfo.ulWidth; > > cuinfo.ulTargetHeight = cuinfo.ulHeight; > > > > @@ -653,6 +657,36 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu > > return 0; > > } > > > > +static int convert_to_cuda_video_chroma_format(enum AVPixelFormat pix_fmt, > > + cudaVideoChromaFormat *out) > > +{ > > + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt); > > + if (!(out && desc && > > + (desc->nb_components == 1 || desc->nb_components == 3) && > > + (desc->log2_chroma_w < 2 && desc->log2_chroma_h < 2))) > > + { > > + return AVERROR(EINVAL); > > + } > > + > > + if (desc->nb_components == 1) > > + { > > + *out = cudaVideoChromaFormat_Monochrome; > > + } > > + else if (desc->flags == AV_PIX_FMT_FLAG_PLANAR) > > + { > > + *out = ((desc->log2_chroma_w == 0) ? cudaVideoChromaFormat_444 : > > + (desc->log2_chroma_h == 0) ? cudaVideoChromaFormat_422 : > > + cudaVideoChromaFormat_420); > > + } > > + else > > + { > > + return AVERROR(EINVAL); > > + } > > + > > + // unfortunately, 420 is the only one that works: > > + return (*out == cudaVideoChromaFormat_420) ? 0 : AVERROR_EXTERNAL; > > +} > > + > > static av_cold int cuvid_decode_init(AVCodecContext *avctx) > > { > > CuvidContext *ctx = avctx->priv_data; > > @@ -663,12 +697,27 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) > > CUcontext cuda_ctx = NULL; > > CUcontext dummy; > > const AVBitStreamFilter *bsf; > > + cudaVideoChromaFormat probed_chroma_format; > > + int probed_width; > > + int probed_height; > > int ret = 0; > > > > enum AVPixelFormat pix_fmts[3] = { AV_PIX_FMT_CUDA, > > AV_PIX_FMT_NV12, > > AV_PIX_FMT_NONE }; > > > > + enum AVPixelFormat probed_pix_fmt = (avctx->pix_fmt <= 0 ? > > + AV_PIX_FMT_YUV420P : > > + avctx->pix_fmt); > > + > > + ret = convert_to_cuda_video_chroma_format(probed_pix_fmt, &probed_chroma_format); > > + if (ret < 0) { > > + // pixel format is not supported: > > + return ret; > > + } > > + probed_width = avctx->coded_width ? avctx->coded_width : 1280; > > + probed_height = avctx->coded_height ? avctx->coded_height : 720; > > + > > // Accelerated transcoding scenarios with 'ffmpeg' require that the > > // pix_fmt be set to AV_PIX_FMT_CUDA early. The sw_pix_fmt, and the > > // pix_fmt for non-accelerated transcoding, do not need to be correct > > @@ -824,7 +873,10 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) > > if (ret < 0) > > goto error; > > > > - ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo); > > + ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo, > > + probed_chroma_format, > > + probed_width, > > + probed_height); > > if (ret < 0) > > goto error; > > > > -- > > 2.9.2 > > > > > Despite wm4 objections I am resubmitting this patch, because I can > find no other reasonable method for rejecting ffmpeg cuvid decoder > when it can't handle a given video stream. An unreasonable > alternative would be to duplicate the work that cuvid.c does > internally on my application side, and that would be of little benefit > to me and to ffmpeg -- I might as well use the nvenc APIs directly > then. I still see this patch as an improvement. I'm still against this. Also, I have the same problem (due to ffmpeg.c being so low level and essentially unsuitable to support hardware transcoding on arbitrary hardware). I'm telling you you won't get far with this anyway, because you need different filter graphs for sw and hw. I also dislike this because it's complex and requires creating the decoder twice (which it shouldn't do in the first place).
On Sat, 21 Jan 2017 10:35:50 -0700 Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > On Sat, Jan 21, 2017 at 10:27 AM, <pkoshevoy@gmail.com> wrote: > > From: Pavel Koshevoy <pkoshevoy@gmail.com> > > > > Evidently CUVID doesn't support decoding 422 or 444 chroma formats, > > and only a limited set of resolutions per codec are supported. > > > > Unfortunately CUVID silently drops packets for video of unsupported > > resolution. However, it will error-out at cuvidCreateDecoder call > > if the indicated video resolution is not supported. > > > > Given that stream resolution and pixel format are typically known as > > a result of probing it is better to use this information during > > avcodec_open2 call to fail immediately, rather than proceeding to > > decode and never receiving any frames from the decoder nor receiving > > any indication of decode failure. > > --- > > libavcodec/cuvid.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 57 insertions(+), 5 deletions(-) > > > > diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c > > index 8fc713d..f9c29a1 100644 > > --- a/libavcodec/cuvid.c > > +++ b/libavcodec/cuvid.c > > @@ -612,7 +612,11 @@ static av_cold int cuvid_decode_end(AVCodecContext *avctx) > > return 0; > > } > > > > -static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cuparseinfo) > > +static int cuvid_test_dummy_decoder(AVCodecContext *avctx, > > + const CUVIDPARSERPARAMS *cuparseinfo, > > + cudaVideoChromaFormat probed_chroma_format, > > + int probed_width, > > + int probed_height) > > { > > CuvidContext *ctx = avctx->priv_data; > > CUVIDDECODECREATEINFO cuinfo; > > @@ -622,11 +626,11 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu > > memset(&cuinfo, 0, sizeof(cuinfo)); > > > > cuinfo.CodecType = cuparseinfo->CodecType; > > - cuinfo.ChromaFormat = cudaVideoChromaFormat_420; > > + cuinfo.ChromaFormat = probed_chroma_format; > > cuinfo.OutputFormat = cudaVideoSurfaceFormat_NV12; > > > > - cuinfo.ulWidth = 1280; > > - cuinfo.ulHeight = 720; > > + cuinfo.ulWidth = probed_width; > > + cuinfo.ulHeight = probed_height; > > cuinfo.ulTargetWidth = cuinfo.ulWidth; > > cuinfo.ulTargetHeight = cuinfo.ulHeight; > > > > @@ -653,6 +657,36 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu > > return 0; > > } > > > > +static int convert_to_cuda_video_chroma_format(enum AVPixelFormat pix_fmt, > > + cudaVideoChromaFormat *out) > > +{ > > + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt); > > + if (!(out && desc && > > + (desc->nb_components == 1 || desc->nb_components == 3) && > > + (desc->log2_chroma_w < 2 && desc->log2_chroma_h < 2))) > > + { > > + return AVERROR(EINVAL); > > + } > > + > > + if (desc->nb_components == 1) > > + { > > + *out = cudaVideoChromaFormat_Monochrome; > > + } > > + else if (desc->flags == AV_PIX_FMT_FLAG_PLANAR) > > + { > > + *out = ((desc->log2_chroma_w == 0) ? cudaVideoChromaFormat_444 : > > + (desc->log2_chroma_h == 0) ? cudaVideoChromaFormat_422 : > > + cudaVideoChromaFormat_420); > > + } > > + else > > + { > > + return AVERROR(EINVAL); > > + } > > + > > + // unfortunately, 420 is the only one that works: > > + return (*out == cudaVideoChromaFormat_420) ? 0 : AVERROR_EXTERNAL; > > +} > > + > > static av_cold int cuvid_decode_init(AVCodecContext *avctx) > > { > > CuvidContext *ctx = avctx->priv_data; > > @@ -663,12 +697,27 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) > > CUcontext cuda_ctx = NULL; > > CUcontext dummy; > > const AVBitStreamFilter *bsf; > > + cudaVideoChromaFormat probed_chroma_format; > > + int probed_width; > > + int probed_height; > > int ret = 0; > > > > enum AVPixelFormat pix_fmts[3] = { AV_PIX_FMT_CUDA, > > AV_PIX_FMT_NV12, > > AV_PIX_FMT_NONE }; > > > > + enum AVPixelFormat probed_pix_fmt = (avctx->pix_fmt <= 0 ? > > + AV_PIX_FMT_YUV420P : > > + avctx->pix_fmt); > > + > > + ret = convert_to_cuda_video_chroma_format(probed_pix_fmt, &probed_chroma_format); > > + if (ret < 0) { > > + // pixel format is not supported: > > + return ret; > > + } > > + probed_width = avctx->coded_width ? avctx->coded_width : 1280; > > + probed_height = avctx->coded_height ? avctx->coded_height : 720; > > + > > // Accelerated transcoding scenarios with 'ffmpeg' require that the > > // pix_fmt be set to AV_PIX_FMT_CUDA early. The sw_pix_fmt, and the > > // pix_fmt for non-accelerated transcoding, do not need to be correct > > @@ -824,7 +873,10 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) > > if (ret < 0) > > goto error; > > > > - ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo); > > + ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo, > > + probed_chroma_format, > > + probed_width, > > + probed_height); > > if (ret < 0) > > goto error; > > > > -- > > 2.9.2 > > > > > Despite wm4 objections I am resubmitting this patch, because I can > find no other reasonable method for rejecting ffmpeg cuvid decoder > when it can't handle a given video stream. An unreasonable > alternative would be to duplicate the work that cuvid.c does > internally on my application side, and that would be of little benefit > to me and to ffmpeg -- I might as well use the nvenc APIs directly > then. I still see this patch as an improvement. PS: Actually, cuvid is a bit different from normal hwaccel and I confused that (blame Sunday "morning"). In this case, do you just want the cuvid to gracefully exit the ffmpeg.c process if decoding is not supported? In that case, this would be much simpler: 1. Return a special error code from cuvid that signals unsupported format 2. Make ffmpeg.c check for this error code, and stop transcoding with an error if it's encountered
On 21/01/17 17:35, Pavel Koshevoy wrote: > On Sat, Jan 21, 2017 at 10:27 AM, <pkoshevoy@gmail.com> wrote: >> From: Pavel Koshevoy <pkoshevoy@gmail.com> >> >> Evidently CUVID doesn't support decoding 422 or 444 chroma formats, >> and only a limited set of resolutions per codec are supported. >> >> Unfortunately CUVID silently drops packets for video of unsupported >> resolution. However, it will error-out at cuvidCreateDecoder call >> if the indicated video resolution is not supported. >> >> Given that stream resolution and pixel format are typically known as >> a result of probing it is better to use this information during >> avcodec_open2 call to fail immediately, rather than proceeding to >> decode and never receiving any frames from the decoder nor receiving >> any indication of decode failure. >> --- >> libavcodec/cuvid.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++----- >> 1 file changed, 57 insertions(+), 5 deletions(-) >> >> diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c >> index 8fc713d..f9c29a1 100644 >> --- a/libavcodec/cuvid.c >> +++ b/libavcodec/cuvid.c >> @@ -612,7 +612,11 @@ static av_cold int cuvid_decode_end(AVCodecContext *avctx) >> return 0; >> } >> >> -static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cuparseinfo) >> +static int cuvid_test_dummy_decoder(AVCodecContext *avctx, >> + const CUVIDPARSERPARAMS *cuparseinfo, >> + cudaVideoChromaFormat probed_chroma_format, >> + int probed_width, >> + int probed_height) >> { >> CuvidContext *ctx = avctx->priv_data; >> CUVIDDECODECREATEINFO cuinfo; >> @@ -622,11 +626,11 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu >> memset(&cuinfo, 0, sizeof(cuinfo)); >> >> cuinfo.CodecType = cuparseinfo->CodecType; >> - cuinfo.ChromaFormat = cudaVideoChromaFormat_420; >> + cuinfo.ChromaFormat = probed_chroma_format; >> cuinfo.OutputFormat = cudaVideoSurfaceFormat_NV12; >> >> - cuinfo.ulWidth = 1280; >> - cuinfo.ulHeight = 720; >> + cuinfo.ulWidth = probed_width; >> + cuinfo.ulHeight = probed_height; >> cuinfo.ulTargetWidth = cuinfo.ulWidth; >> cuinfo.ulTargetHeight = cuinfo.ulHeight; >> >> @@ -653,6 +657,36 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu >> return 0; >> } >> >> +static int convert_to_cuda_video_chroma_format(enum AVPixelFormat pix_fmt, >> + cudaVideoChromaFormat *out) >> +{ >> + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt); >> + if (!(out && desc && >> + (desc->nb_components == 1 || desc->nb_components == 3) && >> + (desc->log2_chroma_w < 2 && desc->log2_chroma_h < 2))) >> + { >> + return AVERROR(EINVAL); >> + } >> + >> + if (desc->nb_components == 1) >> + { >> + *out = cudaVideoChromaFormat_Monochrome; >> + } >> + else if (desc->flags == AV_PIX_FMT_FLAG_PLANAR) >> + { >> + *out = ((desc->log2_chroma_w == 0) ? cudaVideoChromaFormat_444 : >> + (desc->log2_chroma_h == 0) ? cudaVideoChromaFormat_422 : >> + cudaVideoChromaFormat_420); >> + } >> + else >> + { >> + return AVERROR(EINVAL); >> + } >> + >> + // unfortunately, 420 is the only one that works: >> + return (*out == cudaVideoChromaFormat_420) ? 0 : AVERROR_EXTERNAL; >> +} >> + >> static av_cold int cuvid_decode_init(AVCodecContext *avctx) >> { >> CuvidContext *ctx = avctx->priv_data; >> @@ -663,12 +697,27 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) >> CUcontext cuda_ctx = NULL; >> CUcontext dummy; >> const AVBitStreamFilter *bsf; >> + cudaVideoChromaFormat probed_chroma_format; >> + int probed_width; >> + int probed_height; >> int ret = 0; >> >> enum AVPixelFormat pix_fmts[3] = { AV_PIX_FMT_CUDA, >> AV_PIX_FMT_NV12, >> AV_PIX_FMT_NONE }; >> >> + enum AVPixelFormat probed_pix_fmt = (avctx->pix_fmt <= 0 ? >> + AV_PIX_FMT_YUV420P : >> + avctx->pix_fmt); >> + >> + ret = convert_to_cuda_video_chroma_format(probed_pix_fmt, &probed_chroma_format); >> + if (ret < 0) { >> + // pixel format is not supported: >> + return ret; >> + } >> + probed_width = avctx->coded_width ? avctx->coded_width : 1280; >> + probed_height = avctx->coded_height ? avctx->coded_height : 720; >> + >> // Accelerated transcoding scenarios with 'ffmpeg' require that the >> // pix_fmt be set to AV_PIX_FMT_CUDA early. The sw_pix_fmt, and the >> // pix_fmt for non-accelerated transcoding, do not need to be correct >> @@ -824,7 +873,10 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) >> if (ret < 0) >> goto error; >> >> - ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo); >> + ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo, >> + probed_chroma_format, >> + probed_width, >> + probed_height); >> if (ret < 0) >> goto error; >> >> -- >> 2.9.2 >> > > > Despite wm4 objections I am resubmitting this patch, because I can > find no other reasonable method for rejecting ffmpeg cuvid decoder > when it can't handle a given video stream. An alternative method would be to follow the approach of the qsv decoder. There, it doesn't open anything until enough information has been parsed from the stream to have all the appropriate parameters. That entails making a parser inside the decoder and ensuring that the parser returns all of the information you need to pass to the decoder creation, but it avoids any possibility that the user doesn't supply the necessary information and also allows stream format changes to be handled without external support. Another alternative method would be to add a new API which would test "does decoder X support a stream with Y parameters", which could then be called from your application to answer this sort of question directly. (If the only way to then do that in the CUVID case is to open a dummy decoder then I guess it has to do that, other APIs have query functions for this sort of thing.) > An unreasonable > alternative would be to duplicate the work that cuvid.c does > internally on my application side, and that would be of little benefit > to me and to ffmpeg -- I might as well use the nvenc APIs directly > then. That's fair. > I still see this patch as an improvement. I agree with wm4's sentiments here. - Mark
> Despite wm4 objections I am resubmitting this patch, because I can > find no other reasonable method for rejecting ffmpeg cuvid decoder > when it can't handle a given video stream. An unreasonable > alternative would be to duplicate the work that cuvid.c does > internally on my application side, and that would be of little benefit > to me and to ffmpeg -- I might as well use the nvenc APIs directly > then. I still see this patch as an improvement. Cuvid should still return an error, just not on init, while some stream parameters are still unknown, but when decoding the first (couple) frame(s). If there is a case where it doesn't, I'd much rather fix that.
On 01/22/2017 07:05 AM, Timo Rothenpieler wrote: >> Despite wm4 objections I am resubmitting this patch, because I can >> find no other reasonable method for rejecting ffmpeg cuvid decoder >> when it can't handle a given video stream. An unreasonable >> alternative would be to duplicate the work that cuvid.c does >> internally on my application side, and that would be of little benefit >> to me and to ffmpeg -- I might as well use the nvenc APIs directly >> then. I still see this patch as an improvement. > Cuvid should still return an error, just not on init, while some stream > parameters are still unknown, but when decoding the first (couple) frame(s). > If there is a case where it doesn't, I'd much rather fix that. It didn't return an error. I was testing with 5K and 8K h264 samples mentioned at http://kodi.wiki/view/Samples on a GeForce GT 730 which only supports 4K 8-bit h264 -- ctx->cvdl->cuvidParseVideoData never returned an error, and cuvid_handle_video_sequence callback was never called. The only indication I ever got from cuvid that it won't work was during ctx->cvdl->cuvidCreateDecoder call (after my patch). Pavel.
On 01/22/2017 04:58 AM, wm4 wrote: > On Sat, 21 Jan 2017 10:35:50 -0700 > Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > >> On Sat, Jan 21, 2017 at 10:27 AM, <pkoshevoy@gmail.com> wrote: >>> From: Pavel Koshevoy <pkoshevoy@gmail.com> <snip> >>> - ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo); >>> + ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo, >>> + probed_chroma_format, >>> + probed_width, >>> + probed_height); >>> if (ret < 0) >>> goto error; >>> >>> -- >>> 2.9.2 >>> >> >> Despite wm4 objections I am resubmitting this patch, because I can >> find no other reasonable method for rejecting ffmpeg cuvid decoder >> when it can't handle a given video stream. An unreasonable >> alternative would be to duplicate the work that cuvid.c does >> internally on my application side, and that would be of little benefit >> to me and to ffmpeg -- I might as well use the nvenc APIs directly >> then. I still see this patch as an improvement. > PS: > > Actually, cuvid is a bit different from normal hwaccel and I confused > that (blame Sunday "morning"). > > In this case, do you just want the cuvid to gracefully exit the ffmpeg.c > process if decoding is not supported? In that case, this would be much > simpler: > > 1. Return a special error code from cuvid that signals unsupported > format > 2. Make ffmpeg.c check for this error code, and stop transcoding with > an error if it's encountered I am not working with ffmpeg.c -- I am calling ffmpeg public APIs from my application. The 1. point above is essentially what my patch does. The real problem is that CUVID only reports an error at cuvidCreateDecoder time, and not later when I attempt to decode 8K or 5K video -- it just swallows packets and never outputs anything (no errors, no callbacks, nothing). Pavel.
On 01/22/2017 05:28 AM, Mark Thompson wrote: > On 21/01/17 17:35, Pavel Koshevoy wrote: >> On Sat, Jan 21, 2017 at 10:27 AM, <pkoshevoy@gmail.com> wrote: >>> From: Pavel Koshevoy <pkoshevoy@gmail.com> >>> >>> Evidently CUVID doesn't support decoding 422 or 444 chroma formats, >>> and only a limited set of resolutions per codec are supported. >>> >>> Unfortunately CUVID silently drops packets for video of unsupported >>> resolution. However, it will error-out at cuvidCreateDecoder call >>> if the indicated video resolution is not supported. >>> >>> Given that stream resolution and pixel format are typically known as >>> a result of probing it is better to use this information during >>> avcodec_open2 call to fail immediately, rather than proceeding to >>> decode and never receiving any frames from the decoder nor receiving >>> any indication of decode failure. >>> --- >>> libavcodec/cuvid.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++----- >>> 1 file changed, 57 insertions(+), 5 deletions(-) >>> >>> diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c >>> index 8fc713d..f9c29a1 100644 >>> --- a/libavcodec/cuvid.c >>> +++ b/libavcodec/cuvid.c >>> @@ -612,7 +612,11 @@ static av_cold int cuvid_decode_end(AVCodecContext *avctx) >>> return 0; >>> } >>> >>> -static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cuparseinfo) >>> +static int cuvid_test_dummy_decoder(AVCodecContext *avctx, >>> + const CUVIDPARSERPARAMS *cuparseinfo, >>> + cudaVideoChromaFormat probed_chroma_format, >>> + int probed_width, >>> + int probed_height) >>> { >>> CuvidContext *ctx = avctx->priv_data; >>> CUVIDDECODECREATEINFO cuinfo; >>> @@ -622,11 +626,11 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu >>> memset(&cuinfo, 0, sizeof(cuinfo)); >>> >>> cuinfo.CodecType = cuparseinfo->CodecType; >>> - cuinfo.ChromaFormat = cudaVideoChromaFormat_420; >>> + cuinfo.ChromaFormat = probed_chroma_format; >>> cuinfo.OutputFormat = cudaVideoSurfaceFormat_NV12; >>> >>> - cuinfo.ulWidth = 1280; >>> - cuinfo.ulHeight = 720; >>> + cuinfo.ulWidth = probed_width; >>> + cuinfo.ulHeight = probed_height; >>> cuinfo.ulTargetWidth = cuinfo.ulWidth; >>> cuinfo.ulTargetHeight = cuinfo.ulHeight; >>> >>> @@ -653,6 +657,36 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu >>> return 0; >>> } >>> >>> +static int convert_to_cuda_video_chroma_format(enum AVPixelFormat pix_fmt, >>> + cudaVideoChromaFormat *out) >>> +{ >>> + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt); >>> + if (!(out && desc && >>> + (desc->nb_components == 1 || desc->nb_components == 3) && >>> + (desc->log2_chroma_w < 2 && desc->log2_chroma_h < 2))) >>> + { >>> + return AVERROR(EINVAL); >>> + } >>> + >>> + if (desc->nb_components == 1) >>> + { >>> + *out = cudaVideoChromaFormat_Monochrome; >>> + } >>> + else if (desc->flags == AV_PIX_FMT_FLAG_PLANAR) >>> + { >>> + *out = ((desc->log2_chroma_w == 0) ? cudaVideoChromaFormat_444 : >>> + (desc->log2_chroma_h == 0) ? cudaVideoChromaFormat_422 : >>> + cudaVideoChromaFormat_420); >>> + } >>> + else >>> + { >>> + return AVERROR(EINVAL); >>> + } >>> + >>> + // unfortunately, 420 is the only one that works: >>> + return (*out == cudaVideoChromaFormat_420) ? 0 : AVERROR_EXTERNAL; >>> +} >>> + >>> static av_cold int cuvid_decode_init(AVCodecContext *avctx) >>> { >>> CuvidContext *ctx = avctx->priv_data; >>> @@ -663,12 +697,27 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) >>> CUcontext cuda_ctx = NULL; >>> CUcontext dummy; >>> const AVBitStreamFilter *bsf; >>> + cudaVideoChromaFormat probed_chroma_format; >>> + int probed_width; >>> + int probed_height; >>> int ret = 0; >>> >>> enum AVPixelFormat pix_fmts[3] = { AV_PIX_FMT_CUDA, >>> AV_PIX_FMT_NV12, >>> AV_PIX_FMT_NONE }; >>> >>> + enum AVPixelFormat probed_pix_fmt = (avctx->pix_fmt <= 0 ? >>> + AV_PIX_FMT_YUV420P : >>> + avctx->pix_fmt); >>> + >>> + ret = convert_to_cuda_video_chroma_format(probed_pix_fmt, &probed_chroma_format); >>> + if (ret < 0) { >>> + // pixel format is not supported: >>> + return ret; >>> + } >>> + probed_width = avctx->coded_width ? avctx->coded_width : 1280; >>> + probed_height = avctx->coded_height ? avctx->coded_height : 720; >>> + >>> // Accelerated transcoding scenarios with 'ffmpeg' require that the >>> // pix_fmt be set to AV_PIX_FMT_CUDA early. The sw_pix_fmt, and the >>> // pix_fmt for non-accelerated transcoding, do not need to be correct >>> @@ -824,7 +873,10 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) >>> if (ret < 0) >>> goto error; >>> >>> - ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo); >>> + ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo, >>> + probed_chroma_format, >>> + probed_width, >>> + probed_height); >>> if (ret < 0) >>> goto error; >>> >>> -- >>> 2.9.2 >>> >> >> Despite wm4 objections I am resubmitting this patch, because I can >> find no other reasonable method for rejecting ffmpeg cuvid decoder >> when it can't handle a given video stream. > An alternative method would be to follow the approach of the qsv decoder. There, it doesn't open anything until enough information has been parsed from the stream to have all the appropriate parameters. That entails making a parser inside the decoder and ensuring that the parser returns all of the information you need to pass to the decoder creation, but it avoids any possibility that the user doesn't supply the necessary information and also allows stream format changes to be handled without external support. > That sounds interesting, although it's an overkill for my problem -- I already have the probed stream codec parameters, and it is wasteful not to use them to initialize the codec. Your proposed solution sounds robust, but also more difficult to implement. I may attempt it (I'll have to see how QSV does it), but I my patch would still be an improvement if it were applied now. Pavel.
On Sat, 21 Jan 2017 10:27:30 -0700 pkoshevoy@gmail.com wrote: > From: Pavel Koshevoy <pkoshevoy@gmail.com> > > Evidently CUVID doesn't support decoding 422 or 444 chroma formats, > and only a limited set of resolutions per codec are supported. > > Unfortunately CUVID silently drops packets for video of unsupported > resolution. However, it will error-out at cuvidCreateDecoder call > if the indicated video resolution is not supported. > > Given that stream resolution and pixel format are typically known as > a result of probing it is better to use this information during > avcodec_open2 call to fail immediately, rather than proceeding to > decode and never receiving any frames from the decoder nor receiving > any indication of decode failure. So I'm confused. I agree that these errors do not emerge early enough to induce failure at decoder init time, but they should all trigger errors at first-frame decode time. That's certainly true for the chroma format error, because I added that logic. Assuming that is true, you should just be dealing with first-frame failure and then falling back to SW decoding, or whatever policy you want in your app (I assume 8K SW decoding won't go well...). Sure, it's not great that you can't get failures at init time, but you have to be prepared to encounter fatal errors on frame decode with dealing with hardware decoders. Most, if not all, have failure cases that only emerge when decoding a real frame. There's a useful discussion to have about clearly indicating how fatal an error is from that frame decode failure (so we know that it's due to unsupported characteristics rather than a single bad frame), but this change won't help us get there. And I'll repeat wm4's warning that these tests depend on the presence of probed data that cannot be assumed in the general case; it may work for your app, but other applications may not fill in those fields, so it cannot be considered reliable functionality. Thanks, --phil
On Sun, Jan 22, 2017 at 9:37 AM, Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > On 01/22/2017 07:05 AM, Timo Rothenpieler wrote: >>> >>> Despite wm4 objections I am resubmitting this patch, because I can >>> find no other reasonable method for rejecting ffmpeg cuvid decoder >>> when it can't handle a given video stream. An unreasonable >>> alternative would be to duplicate the work that cuvid.c does >>> internally on my application side, and that would be of little benefit >>> to me and to ffmpeg -- I might as well use the nvenc APIs directly >>> then. I still see this patch as an improvement. >> >> Cuvid should still return an error, just not on init, while some stream >> parameters are still unknown, but when decoding the first (couple) >> frame(s). >> If there is a case where it doesn't, I'd much rather fix that. > > > > It didn't return an error. I was testing with 5K and 8K h264 samples > mentioned at http://kodi.wiki/view/Samples on a GeForce GT 730 which only > supports 4K 8-bit h264 -- ctx->cvdl->cuvidParseVideoData never returned an > error, and cuvid_handle_video_sequence callback was never called. The only > indication I ever got from cuvid that it won't work was during > ctx->cvdl->cuvidCreateDecoder call (after my patch). > I've just tested on GeForce GTX 1060 with PATAGONIA 8K mp4 sample and I see the same behavior -- packets are accepted but the callbacks are not called. With the Snow Monkeys in Japan 5K Retina 60p (Ultra HD).mp4 on GeForce 1060 it actually does call the callbacks and logs errors, so that's different from GeForce 730: [h264_cuvid @ 0x7fff9c000980] ctx->cvdl->cuvidDecodePicture(ctx->cudecoder, picparams) failed -> CUDA_ERROR_INVALID_HANDLE: invalid resource handle [h264_cuvid @ 0x7fff9c000980] cuvid decode callback error Pavel.
On 1/22/17 11:13 AM, Philip Langdale wrote: > On Sat, 21 Jan 2017 10:27:30 -0700 > pkoshevoy@gmail.com wrote: > >> From: Pavel Koshevoy <pkoshevoy@gmail.com> >> >> Evidently CUVID doesn't support decoding 422 or 444 chroma formats, >> and only a limited set of resolutions per codec are supported. >> >> Unfortunately CUVID silently drops packets for video of unsupported >> resolution. However, it will error-out at cuvidCreateDecoder call >> if the indicated video resolution is not supported. >> >> Given that stream resolution and pixel format are typically known as >> a result of probing it is better to use this information during >> avcodec_open2 call to fail immediately, rather than proceeding to >> decode and never receiving any frames from the decoder nor receiving >> any indication of decode failure. > So I'm confused. I agree that these errors do not emerge early enough > to induce failure at decoder init time, but they should all trigger > errors at first-frame decode time. That's certainly true for the > chroma format error, because I added that logic. > > Assuming that is true, you should just be dealing with first-frame > failure and then falling back to SW decoding, or whatever policy > you want in your app (I assume 8K SW decoding won't go well...). > Yeah, but I don't even get a 1st frame failure with YUV420P 8K from h264_cuvid -- all I ever get is a 0 from avcodec_send_packet and AVERROR(EAGAIN) from avcodec_receive_frame. This is what I've observed with GeForce GT 730 and GeForce GTX 1060 on openSUSE 42.2 and Ubuntu 16.04 with latest nvidia drivers. I will try to make my application more robust too -- I will have to buffer all the packets sent to the decoder until the 1st frame either succeeds or fails, so I can re-send the same packets to another decoder in case of 1st frame failure. Pavel.
On Sun, 22 Jan 2017 12:12:26 -0700 Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > Yeah, but I don't even get a 1st frame failure with YUV420P 8K from > h264_cuvid -- all I ever get is a 0 from avcodec_send_packet and > AVERROR(EAGAIN) from avcodec_receive_frame. This is what I've > observed with GeForce GT 730 and GeForce GTX 1060 on openSUSE 42.2 > and Ubuntu 16.04 with latest nvidia drivers. > I tested the 8K h.264 sample, and my experience is the same as yours. It really does just stall out. Given that, it looks like the dimensions check can only be done in the way that you've done it. But we shouldn't add the chroma format check; we already have the check after the parser calls handle_video_sequence so that the first frame will fail - and that's reliable because it uses the cuvid parser and doesn't rely on the externally probed values. So, can you update the patch to just check the dimensions? > I will try to make my application more robust too -- I will have to > buffer all the packets sent to the decoder until the 1st frame either > succeeds or fails, so I can re-send the same packets to another > decoder in case of 1st frame failure. Yeah. It's not fun but it is necessary. wm4 did this for mpv. Thanks, --phil
diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c index 8fc713d..f9c29a1 100644 --- a/libavcodec/cuvid.c +++ b/libavcodec/cuvid.c @@ -612,7 +612,11 @@ static av_cold int cuvid_decode_end(AVCodecContext *avctx) return 0; } -static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cuparseinfo) +static int cuvid_test_dummy_decoder(AVCodecContext *avctx, + const CUVIDPARSERPARAMS *cuparseinfo, + cudaVideoChromaFormat probed_chroma_format, + int probed_width, + int probed_height) { CuvidContext *ctx = avctx->priv_data; CUVIDDECODECREATEINFO cuinfo; @@ -622,11 +626,11 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu memset(&cuinfo, 0, sizeof(cuinfo)); cuinfo.CodecType = cuparseinfo->CodecType; - cuinfo.ChromaFormat = cudaVideoChromaFormat_420; + cuinfo.ChromaFormat = probed_chroma_format; cuinfo.OutputFormat = cudaVideoSurfaceFormat_NV12; - cuinfo.ulWidth = 1280; - cuinfo.ulHeight = 720; + cuinfo.ulWidth = probed_width; + cuinfo.ulHeight = probed_height; cuinfo.ulTargetWidth = cuinfo.ulWidth; cuinfo.ulTargetHeight = cuinfo.ulHeight; @@ -653,6 +657,36 @@ static int cuvid_test_dummy_decoder(AVCodecContext *avctx, CUVIDPARSERPARAMS *cu return 0; } +static int convert_to_cuda_video_chroma_format(enum AVPixelFormat pix_fmt, + cudaVideoChromaFormat *out) +{ + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt); + if (!(out && desc && + (desc->nb_components == 1 || desc->nb_components == 3) && + (desc->log2_chroma_w < 2 && desc->log2_chroma_h < 2))) + { + return AVERROR(EINVAL); + } + + if (desc->nb_components == 1) + { + *out = cudaVideoChromaFormat_Monochrome; + } + else if (desc->flags == AV_PIX_FMT_FLAG_PLANAR) + { + *out = ((desc->log2_chroma_w == 0) ? cudaVideoChromaFormat_444 : + (desc->log2_chroma_h == 0) ? cudaVideoChromaFormat_422 : + cudaVideoChromaFormat_420); + } + else + { + return AVERROR(EINVAL); + } + + // unfortunately, 420 is the only one that works: + return (*out == cudaVideoChromaFormat_420) ? 0 : AVERROR_EXTERNAL; +} + static av_cold int cuvid_decode_init(AVCodecContext *avctx) { CuvidContext *ctx = avctx->priv_data; @@ -663,12 +697,27 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) CUcontext cuda_ctx = NULL; CUcontext dummy; const AVBitStreamFilter *bsf; + cudaVideoChromaFormat probed_chroma_format; + int probed_width; + int probed_height; int ret = 0; enum AVPixelFormat pix_fmts[3] = { AV_PIX_FMT_CUDA, AV_PIX_FMT_NV12, AV_PIX_FMT_NONE }; + enum AVPixelFormat probed_pix_fmt = (avctx->pix_fmt <= 0 ? + AV_PIX_FMT_YUV420P : + avctx->pix_fmt); + + ret = convert_to_cuda_video_chroma_format(probed_pix_fmt, &probed_chroma_format); + if (ret < 0) { + // pixel format is not supported: + return ret; + } + probed_width = avctx->coded_width ? avctx->coded_width : 1280; + probed_height = avctx->coded_height ? avctx->coded_height : 720; + // Accelerated transcoding scenarios with 'ffmpeg' require that the // pix_fmt be set to AV_PIX_FMT_CUDA early. The sw_pix_fmt, and the // pix_fmt for non-accelerated transcoding, do not need to be correct @@ -824,7 +873,10 @@ static av_cold int cuvid_decode_init(AVCodecContext *avctx) if (ret < 0) goto error; - ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo); + ret = cuvid_test_dummy_decoder(avctx, &ctx->cuparseinfo, + probed_chroma_format, + probed_width, + probed_height); if (ret < 0) goto error;
From: Pavel Koshevoy <pkoshevoy@gmail.com> Evidently CUVID doesn't support decoding 422 or 444 chroma formats, and only a limited set of resolutions per codec are supported. Unfortunately CUVID silently drops packets for video of unsupported resolution. However, it will error-out at cuvidCreateDecoder call if the indicated video resolution is not supported. Given that stream resolution and pixel format are typically known as a result of probing it is better to use this information during avcodec_open2 call to fail immediately, rather than proceeding to decode and never receiving any frames from the decoder nor receiving any indication of decode failure. --- libavcodec/cuvid.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 5 deletions(-)