[FFmpeg-devel] [PATCH] hwcontext_vdpau: implement av_hwdevice_get_hwframe_constraints()

[wm4]

On Mon, 15 Jan 2018 11:17:39 +0000
Mark Thompson <sw at 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.