[FFmpeg-devel] [PATCH 2/2] vaapi_h265: Add named options for setting profile and level

[Song, Ruiling]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of
> Hendrik Leppkes
> Sent: Thursday, November 30, 2017 6:12 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/2] vaapi_h265: Add named options for
> setting profile and level
> 
> On Thu, Nov 30, 2017 at 10:40 AM, Li, Zhong <zhong.li at intel.com> wrote:
> >> -----Original Message-----
> >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> >> Of Mark Thompson
> >> Sent: Thursday, November 30, 2017 7:30 AM
> >> To: ffmpeg-devel at ffmpeg.org
> >> Subject: [FFmpeg-devel] [PATCH 2/2] vaapi_h265: Add named options for
> >> setting profile and level
> >>
> >> Also fixes the default, which previously contained a nonsense value.
> >> ---
> >> On 29/11/17 03:51, Li, Zhong wrote:
> >> >> On 28/11/17 07:46, Ruiling Song wrote:
> >> >>> Signed-off-by: Ruiling Song <ruiling.song at intel.com>
> >> >>> ---
> >> >>>  libavcodec/vaapi_encode_h265.c | 2 +-
> >> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>>
> >> >>> diff --git a/libavcodec/vaapi_encode_h265.c
> >> >>> b/libavcodec/vaapi_encode_h265.c index 3ae92a7..32b8bc6 100644
> >> >>> --- a/libavcodec/vaapi_encode_h265.c
> >> >>> +++ b/libavcodec/vaapi_encode_h265.c
> >> >>> @@ -219,7 +219,7 @@ static int
> >> >> vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
> >> >>>          .general_non_packed_constraint_flag = 1,
> >> >>>          .general_frame_only_constraint_flag = 1,
> >> >>>
> >> >>> -        .general_level_idc     = avctx->level,
> >> >>> +        .general_level_idc     = avctx->level * 3,
> >> >>>      };
> >> >>>
> >> >>> vps->profile_tier_level.general_profile_compatibility_flag[avctx->pr
> >> >>> vps->of
> >> >>> ile & 31] = 1;
> >> >>>
> >> >>>
> >> >> The documentation has always said "profile and level set the values
> >> >> of general_profile_idc and general_level_idc respectively"
> >> >> (<http://ffmpeg.org/ffmpeg-codecs.html#VAAPI-encoders>) - the code
> >> >> disagreed for a while, but that was made consistent in
> >> >> 00179664bccd1dd6fa0d1c40db453528757bf6f7.
> >> >>
> >> >> Why do you want to change it to be general_level_idc / 3 instead?
> >> >
> >> > As HEVC spec, "general_level_idc and sub_layer_level_idc[ i ] shall be
> >> > set equal to a value of 30 times the level number specified in Table A.4."
> >> > And use the tool "mediainfo" to check the encoded bitstream, it show the
> >> level is 1.7 if set the option "-level" to be 51.
> >> >
> >> > Maybe the documentation
> >> ((<http://ffmpeg.org/ffmpeg-codecs.html#VAAPI-encoders>)) also needs to
> >> be changed?
> >>
> >> Hmm, right - the default is wrong, so you get a nonsense value there.
> >>
> >> How about we fix that and add named constants to make it clearer, like this?
> >
> > I think it is a good idea, except one situation：
> > Suppose user set the level to be 41, they may want an encoded bitstream with
> level 4.1, right?
> > But actually the output level is 1.4 (using mediainfo to check this), currently we
> are forcing them to set the option to be 4.1 or 123, right?
> > Haven't verify your patch, just infer from the code.
> 
> 
> But 41 is never 4.1 for HEVC, so using a scheme that was basically
> "invented" doesn't seem right (probably as a carry-over from H264).
> When decoding, avctx->level contains 123, not 41, for example, or for
> a closer match, NVENC accepts -profile:v 123 or -profile:v 4.1, but
> not 41.
> I think its more important to stay consistent here (both to the HEVC
> spec and other HEVC encoders), instead of catering to an imaginary
> value that is never actually used in relation to HEVC.

I agree with you on this. It would be better if we can keep consistent here.
For this level information, if user want to set level to 5.1 in hevc.
They can do as: "-level 5.1" or "-level 153". I tried Mark's patch, it works as expected.

- Ruiling

> 
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel