[FFmpeg-devel] [PATCH] lavc/vaapi_decode: add va_profile format map support for HEVC_REXT

[Fu, Linjie]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: Friday, March 29, 2019 07:34
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/vaapi_decode: add va_profile
> format map support for HEVC_REXT
> 
> On 28/03/2019 04:03, Linjie Fu wrote:
> > HEVC_REXT will be map to {VAProfileHEVCMain422_10,
> VAProfileHEVCMain444,
> > VAProfileHEVCMain444_10} in vaapi_profile_map[], since need to be
> distinguished
> > to select the exact va_profile.
> >
> > Add va_profile -> AV_PIX_FMT map for FF_PROFILE_HEVC_REXT to match
> the
> > exact va_profile.
> >
> > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > ---
> >  libavcodec/vaapi_decode.c | 29 +++++++++++++++++++++++++----
> >  1 file changed, 25 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
> > index 015154b879..1cb8683b7c 100644
> > --- a/libavcodec/vaapi_decode.c
> > +++ b/libavcodec/vaapi_decode.c
> > @@ -414,6 +414,18 @@ static const struct {
> >  #undef MAP
> >  };
> >
> > +static const struct {
> > +    VAProfile va_profile;
> > +    enum AVPixelFormat pix_fmt;
> > +} rext_format_map[] = {
> > +#define MAP(vp, av) { VAProfileHEVCMain ## vp, AV_PIX_FMT_ ## av }
> > +    MAP(422_10,  YUYV422),
> > +    MAP(422_10,  YUV422P10LE),
> > +    MAP(444,     YUV444P),
> > +    MAP(444_10,  YUV444P10LE),
> 
> This doesn't work - you can't guess the rext profile from the chroma format
> and bit depth information, because the profiles are all overlapping (see table
> A.1).
> 
> You need to use the profile constraint flags to determine it - this lookup is
> implemented by ff_h265_get_profile(), but you'll need to extract all the flags
> to put into it.
> 
Thanks for the advice Mark, and I think these constraint flags should be parsed firstly in decode_profile_tier_level():
https://github.com/FFmpeg/FFmpeg/blob/9dece050ef01c70df1ac74a04da3548b3c0d79a9/libavcodec/hevc_ps.c#L265

Currently, they are skipped when parsing the sps:
    skip_bits(gb, 16); // XXX_reserved_zero_44bits[0..15]
    skip_bits(gb, 16); // XXX_reserved_zero_44bits[16..31]
    skip_bits(gb, 12); // XXX_reserved_zero_44bits[32..43]
Will it be acceptable to add these flags in common hevc parser function for the usage in VAAPI decode? 

> > +#undef MAP
> > +};
> > +
> >  /*
> >   * Set *va_config and the frames_ref fields from the current codec
> parameters
> >   * in avctx.
> > @@ -426,7 +438,7 @@ static int
> vaapi_decode_make_config(AVCodecContext *avctx,
> >      AVVAAPIHWConfig       *hwconfig    = NULL;
> >      AVHWFramesConstraints *constraints = NULL;
> >      VAStatus vas;
> > -    int err, i, j;
> > +    int err, i, j, k;
> >      const AVCodecDescriptor *codec_desc;
> >      VAProfile *profile_list = NULL, matched_va_profile;
> >      int profile_count, exact_match, matched_ff_profile;
> > @@ -467,13 +479,22 @@ static int
> vaapi_decode_make_config(AVCodecContext *avctx,
> >          if (avctx->profile == vaapi_profile_map[i].codec_profile ||
> >              vaapi_profile_map[i].codec_profile == FF_PROFILE_UNKNOWN)
> >              profile_match = 1;
> > -        for (j = 0; j < profile_count; j++) {
> > -            if (vaapi_profile_map[i].va_profile == profile_list[j]) {
> > +        if (avctx->profile == FF_PROFILE_HEVC_REXT) {
> > +            /* find the exact va_profile for HEVC_REXT */
> > +            for (j = 0; j < FF_ARRAY_ELEMS(rext_format_map); j++) {
> > +                if (avctx->pix_fmt == rext_format_map[j].pix_fmt)
> > +                   break;
> > +            }
> > +            if (vaapi_profile_map[i].va_profile !=
> rext_format_map[j].va_profile)
> > +                continue;
> > +        }
> 
> Codec-specific stuff probably shouldn't be hidden in the middle of the
> generic code like this.

Got your point and I add this to avoid redundant map query, for other codecs, 
codec_id and codec_profile are enough to determine the exact va_profile.
Any advice for coping with this situation?
(Did you mean finding some other methods to modify in vaapi_hevc.c, because all
 vaapi decoders are sharing same init funcion)

> > +        for (k = 0; k < profile_count; k++) {
> > +            if (vaapi_profile_map[i].va_profile == profile_list[k]) {
> >                  exact_match = profile_match;
> >                  break;
> >              }
> >          }
> > -        if (j < profile_count) {
> > +        if (k < profile_count) {
> >              matched_va_profile = vaapi_profile_map[i].va_profile;
> >              matched_ff_profile = vaapi_profile_map[i].codec_profile;
> >              if (exact_match)
> >
> 
> When will it be possible to get hardware which supports these profiles?
> 

Honestly, I don't know when the hardware (ICL) will be available, either.

Thanks very much.
linjie