On Mon, 15 Jan 2018 11:34:30 +0000
Mark Thompson <s...@jkqxz.net> wrote:
> On 15/01/18 11:22, wm4 wrote:
> > On Mon, 15 Jan 2018 11:17:39 +0000
> > Mark Thompson <s...@jkqxz.net> wrote:
> >
> >> On 13/01/18 06:06, wm4 wrote:
> >>> In addition, this does not allow creating frames contexts with sw_format
> >>> for which no known transfer formats exist. In theory, we should check
> >>> whether the chroma format (i.e. the sw_format) is supported at all by
> >>> the vdpau driver, but checking for transfer formats has the same effect.
> >>>
> >>> Note that the pre-existing code adds 1 to priv->nb_pix_fmts[i] for
> >>> unknown reason, and some checks need to account for that to check for
> >>> empty lists. They are not off-by-one errors.
> >>> ---
> >>> libavutil/hwcontext_vdpau.c | 55
> >>> ++++++++++++++++++++++++++++++++-------------
> >>> 1 file changed, 39 insertions(+), 16 deletions(-)
> >>>
> >>> diff --git a/libavutil/hwcontext_vdpau.c b/libavutil/hwcontext_vdpau.c
> >>> index 9b8f839647..c11c3cfdab 100644
> >>> --- a/libavutil/hwcontext_vdpau.c
> >>> +++ b/libavutil/hwcontext_vdpau.c
> >>> @@ -79,11 +79,12 @@ static const VDPAUPixFmtMap pix_fmts_444[] = {
> >>>
> >>> static const struct {
> >>> VdpChromaType chroma_type;
> >>> + enum AVPixelFormat frames_sw_format;
> >>> const VDPAUPixFmtMap *map;
> >>> } vdpau_pix_fmts[] = {
> >>> - { VDP_CHROMA_TYPE_420, pix_fmts_420 },
> >>> - { VDP_CHROMA_TYPE_422, pix_fmts_422 },
> >>> - { VDP_CHROMA_TYPE_444, pix_fmts_444 },
> >>> + { VDP_CHROMA_TYPE_420, AV_PIX_FMT_YUV420P, pix_fmts_420 },
> >>> + { VDP_CHROMA_TYPE_422, AV_PIX_FMT_YUV422P, pix_fmts_422 },
> >>> + { VDP_CHROMA_TYPE_444, AV_PIX_FMT_YUV444P, pix_fmts_444 },
> >>> };
> >>>
> >>> static int count_pixfmts(const VDPAUPixFmtMap *map)
> >>> @@ -170,6 +171,35 @@ static void vdpau_device_uninit(AVHWDeviceContext
> >>> *ctx)
> >>> av_freep(&priv->pix_fmts[i]);
> >>> }
> >>>
> >>> +static int vdpau_frames_get_constraints(AVHWDeviceContext *ctx,
> >>> + const void *hwconfig,
> >>> + AVHWFramesConstraints
> >>> *constraints)
> >>> +{
> >>> + VDPAUDeviceContext *priv = ctx->internal->priv;
> >>> + int nb_sw_formats = 0;
> >>> + int i;
> >>> +
> >>> + constraints->valid_sw_formats =
> >>> av_malloc_array(FF_ARRAY_ELEMS(vdpau_pix_fmts) + 1,
> >>> +
> >>> sizeof(*constraints->valid_sw_formats));
> >>> + if (!constraints->valid_sw_formats)
> >>> + return AVERROR(ENOMEM);
> >>> +
> >>> + for (i = 0; i < FF_ARRAY_ELEMS(vdpau_pix_fmts); i++) {
> >>> + if (priv->nb_pix_fmts[i] > 1)
> >>> + constraints->valid_sw_formats[nb_sw_formats++] =
> >>> vdpau_pix_fmts[i].frames_sw_format;
> >>> + }
> >>> + constraints->valid_sw_formats[nb_sw_formats] = AV_PIX_FMT_NONE;
> >>> +
> >>> + constraints->valid_hw_formats = av_malloc_array(2,
> >>> sizeof(*constraints->valid_hw_formats));
> >>> + if (!constraints->valid_hw_formats)
> >>> + return AVERROR(ENOMEM);
> >>> +
> >>> + constraints->valid_hw_formats[0] = AV_PIX_FMT_VDPAU;
> >>> + constraints->valid_hw_formats[1] = AV_PIX_FMT_NONE;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> static void vdpau_buffer_free(void *opaque, uint8_t *data)
> >>> {
> >>> AVHWFramesContext *ctx = opaque;
> >>> @@ -214,26 +244,18 @@ static int vdpau_frames_init(AVHWFramesContext *ctx)
> >>>
> >>> int i;
> >>>
> >>> - switch (ctx->sw_format) {
> >>> - case AV_PIX_FMT_YUV420P: priv->chroma_type = VDP_CHROMA_TYPE_420;
> >>> break;
> >>> - case AV_PIX_FMT_YUV422P: priv->chroma_type = VDP_CHROMA_TYPE_422;
> >>> break;
> >>> - case AV_PIX_FMT_YUV444P: priv->chroma_type = VDP_CHROMA_TYPE_444;
> >>> break;
> >>> - default:
> >>> - av_log(ctx, AV_LOG_ERROR, "Unsupported data layout: %s\n",
> >>> - av_get_pix_fmt_name(ctx->sw_format));
> >>> - return AVERROR(ENOSYS);
> >>> - }
> >>> -
> >>> for (i = 0; i < FF_ARRAY_ELEMS(vdpau_pix_fmts); i++) {
> >>> - if (vdpau_pix_fmts[i].chroma_type == priv->chroma_type) {
> >>> + if (vdpau_pix_fmts[i].frames_sw_format == ctx->sw_format) {
> >>> + priv->chroma_type = vdpau_pix_fmts[i].chroma_type;
> >>> priv->chroma_idx = i;
> >>> priv->pix_fmts = device_priv->pix_fmts[i];
> >>> priv->nb_pix_fmts = device_priv->nb_pix_fmts[i];
> >>> break;
> >>> }
> >>> }
> >>> - if (!priv->pix_fmts) {
> >>> - av_log(ctx, AV_LOG_ERROR, "Unsupported chroma type: %d\n",
> >>> priv->chroma_type);
> >>> + if (priv->nb_pix_fmts < 2) {
> >>
> >> I think keeping the (equivalent) older check of !priv->pix_fmts would look
> >> slightly clearer?
> >
> > I don't think that would be the same. Checking priv->pix_fmts tells you
> > that there was a matching chroma type, but checking nb_pix_fmts < 2
> > tells you in addition whether there is any working sw_format. That in
> > turn is needed to tell whether the vdpau implementation supports the
> > chroma format at all.
>
> Right, priv->pix_fmts could be a valid list containing nothing. Then yes,
> use your way.
Thanks for the review, pushed.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
