[FFmpeg-devel] lavc/h264_levels: add MaxMBPS checking and update fate test.

Submitted by Decai Lin on March 6, 2019, 8:21 a.m.

Details

Message ID 1551860475-25819-1-git-send-email-decai.lin@intel.com
State New
Headers show

Commit Message

Decai Lin March 6, 2019, 8:21 a.m.
1. add MaxMBPS checking for level idc setting to align with AVC spec
   AnnexA table A-1/A-6 level limits.
2. update h264 level fate test.

Signed-off-by: Decai Lin <decai.lin@intel.com>
---
 libavcodec/h264_levels.c       |  5 +++++
 libavcodec/h264_levels.h       |  1 +
 libavcodec/h264_metadata_bsf.c | 14 +++++++++++++-
 libavcodec/tests/h264_levels.c | 41 ++++++++++++++++++++++++++++++++++++++---
 libavcodec/vaapi_encode_h264.c | 13 +++++++++++++
 5 files changed, 70 insertions(+), 4 deletions(-)

Comments

Mark Thompson March 7, 2019, 12:24 a.m.
On 06/03/2019 08:21, Decai Lin wrote:
> 1. add MaxMBPS checking for level idc setting to align with AVC spec
>    AnnexA table A-1/A-6 level limits.
> 2. update h264 level fate test.
> 
> Signed-off-by: Decai Lin <decai.lin@intel.com>
> ---
>  libavcodec/h264_levels.c       |  5 +++++
>  libavcodec/h264_levels.h       |  1 +
>  libavcodec/h264_metadata_bsf.c | 14 +++++++++++++-
>  libavcodec/tests/h264_levels.c | 41 ++++++++++++++++++++++++++++++++++++++---
>  libavcodec/vaapi_encode_h264.c | 13 +++++++++++++
>  5 files changed, 70 insertions(+), 4 deletions(-)

Nice, this is a good idea.  Some comments below.

> diff --git a/libavcodec/h264_levels.c b/libavcodec/h264_levels.c
> index 7a55116..e457dbe 100644
> --- a/libavcodec/h264_levels.c
> +++ b/libavcodec/h264_levels.c
> @@ -89,11 +89,13 @@ const H264LevelDescriptor *ff_h264_get_level(int level_idc,
>  
>  const H264LevelDescriptor *ff_h264_guess_level(int profile_idc,
>                                                 int64_t bitrate,
> +                                               int framerate,
>                                                 int width, int height,
>                                                 int max_dec_frame_buffering)
>  {
>      int width_mbs  = (width  + 15) / 16;
>      int height_mbs = (height + 15) / 16;
> +    int64_t mb_processing_rate = (int64_t) (width_mbs * height_mbs * framerate);

This doesn't work - if it overflows you've already hit undefined behaviour in the calculation before the cast.

>      int no_cs3f = !(profile_idc == 66 ||
>                      profile_idc == 77 ||
>                      profile_idc == 88);
> @@ -108,6 +110,9 @@ const H264LevelDescriptor *ff_h264_guess_level(int profile_idc,
>          if (bitrate > (int64_t)level->max_br * h264_get_br_factor(profile_idc))
>              continue;
>  
> +        if (mb_processing_rate > (int64_t)level->max_mbps)
> +            continue;
> +
>          if (width_mbs  * height_mbs > level->max_fs)
>              continue;
>          if (width_mbs  * width_mbs  > 8 * level->max_fs)
> diff --git a/libavcodec/h264_levels.h b/libavcodec/h264_levels.h
> index 4189fc6..0a0f410 100644
> --- a/libavcodec/h264_levels.h
> +++ b/libavcodec/h264_levels.h
> @@ -46,6 +46,7 @@ const H264LevelDescriptor *ff_h264_get_level(int level_idc,
>   */
>  const H264LevelDescriptor *ff_h264_guess_level(int profile_idc,
>                                                 int64_t bitrate,
> +                                               int framerate,
>                                                 int width, int height,
>                                                 int max_dec_frame_buffering);
>  
> diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
> index a17987a..61c2711 100644
> --- a/libavcodec/h264_metadata_bsf.c
> +++ b/libavcodec/h264_metadata_bsf.c
> @@ -223,6 +223,7 @@ static int h264_metadata_update_sps(AVBSFContext *bsf,
>              const H264LevelDescriptor *desc;
>              int64_t bit_rate;
>              int width, height, dpb_frames;
> +            int framerate, fr_num, fr_den;
>  
>              if (sps->vui.nal_hrd_parameters_present_flag) {
>                  bit_rate = (sps->vui.nal_hrd_parameters.bit_rate_value_minus1[0] + 1) *
> @@ -244,7 +245,18 @@ static int h264_metadata_update_sps(AVBSFContext *bsf,
>              height = 16 * (sps->pic_height_in_map_units_minus1 + 1) *
>                  (2 - sps->frame_mbs_only_flag);
>  
> -            desc = ff_h264_guess_level(sps->profile_idc, bit_rate,
> +            if (ctx->tick_rate.num > 0 && ctx->tick_rate.den > 0)
> +                av_reduce(&fr_num, &fr_den,
> +                        ctx->tick_rate.num, ctx->tick_rate.den, 65535);
> +            else
> +                av_reduce(&fr_num, &fr_den,
> +                        ctx->tick_rate.den, ctx->tick_rate.num, 65535);

Since the VUI fields are set above, I suggest using them instead here because that will include an existing rate in the stream.

Also, the tick rate is only directly usable as fieldrate if fixed_frame_rate_flag is set.

> +            if (fr_den > 0)
> +                framerate = fr_num / fr_den;
> +            else
> +                framerate = fr_num;

I'm not sure what this is trying to do.  If the number isn't valid, the framerate should be passed as zero to avoid constraining anything.

> +
> +            desc = ff_h264_guess_level(sps->profile_idc, bit_rate, framerate,
>                                         width, height, dpb_frames);
>              if (desc) {
>                  level_idc = desc->level_idc;
> diff --git a/libavcodec/tests/h264_levels.c b/libavcodec/tests/h264_levels.c
> index 0e00f05..6176c0b 100644
> --- a/libavcodec/tests/h264_levels.c
> +++ b/libavcodec/tests/h264_levels.c
> @@ -62,6 +62,31 @@ static const struct {
>  static const struct {
>      int width;
>      int height;
> +    int framerate;
> +    int level_idc;
> +} test_framerate[] = {
> +    // Some typical sizes and frame rates.
> +    // (From H.264 table A-1 and table A-6)
> +    {  176,  144,  15, 10 },
> +    {  320,  240,  10, 11 },
> +    {  352,  288,  30, 13 },
> +    {  352,  576,  25, 21 },
> +    {  640,  480,  33, 30 },
> +    {  720,  576,  25, 30 },
> +    { 1024,  768,  35, 31 },
> +    { 1280,  720,  30, 31 },
> +    { 1280, 1024,  42, 32 },
> +    { 1920, 1088,  30, 40 },
> +    { 2048, 1024,  30, 40 },
> +    { 2048, 1088,  60, 42 },
> +    { 3680, 1536,  26, 50 },
> +    { 4096, 2048,  30, 51 },
> +    { 4096, 2160,  60, 52 },

These are all just under the constraint - add a few which are just over the given numbers too.  E.g. 1280x720 at 31fps doesn't fit in level 3.1 so needs 3.2.

> +};
> +
> +static const struct {
> +    int width;
> +    int height;
>      int dpb_size;
>      int level_idc;
>  } test_dpb[] = {
> @@ -147,14 +172,23 @@ int main(void)
>      } while (0)
>  
>      for (i = 0; i < FF_ARRAY_ELEMS(test_sizes); i++) {
> -        level = ff_h264_guess_level(0, 0, test_sizes[i].width,
> +        level = ff_h264_guess_level(0, 0, 0, test_sizes[i].width,
>                                      test_sizes[i].height, 0);
>          CHECK(test_sizes[i].level_idc, "size %dx%d",
>                test_sizes[i].width, test_sizes[i].height);
>      }
>  
> +    for (i = 0; i < FF_ARRAY_ELEMS(test_framerate); i++) {
> +        level = ff_h264_guess_level(0, 0, test_framerate[i].framerate,
> +                                    test_framerate[i].width,
> +                                    test_framerate[i].height, 0);
> +        CHECK(test_framerate[i].level_idc, "framerate %d, size %dx%d",
> +              test_framerate[i].framerate, test_framerate[i].width,
> +              test_framerate[i].height);
> +    }
> +
>      for (i = 0; i < FF_ARRAY_ELEMS(test_dpb); i++) {
> -        level = ff_h264_guess_level(0, 0, test_dpb[i].width,
> +        level = ff_h264_guess_level(0, 0, 0, test_dpb[i].width,
>                                      test_dpb[i].height,
>                                      test_dpb[i].dpb_size);
>          CHECK(test_dpb[i].level_idc, "size %dx%d dpb %d",
> @@ -164,7 +198,7 @@ int main(void)
>  
>      for (i = 0; i < FF_ARRAY_ELEMS(test_bitrate); i++) {
>          level = ff_h264_guess_level(test_bitrate[i].profile_idc,
> -                                    test_bitrate[i].bitrate,
> +                                    test_bitrate[i].bitrate, 0,
>                                      0, 0, 0);
>          CHECK(test_bitrate[i].level_idc, "bitrate %"PRId64" profile %d",
>                test_bitrate[i].bitrate, test_bitrate[i].profile_idc);
> @@ -173,6 +207,7 @@ int main(void)
>      for (i = 0; i < FF_ARRAY_ELEMS(test_all); i++) {
>          level = ff_h264_guess_level(test_all[i].profile_idc,
>                                      test_all[i].bitrate,
> +                                    0,
>                                      test_all[i].width,
>                                      test_all[i].height,
>                                      test_all[i].dpb_frames);
> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> index 91be33f..7e7fd6a 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -329,9 +329,22 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>          sps->level_idc = avctx->level;
>      } else {
>          const H264LevelDescriptor *level;
> +        int framerate, fr_num, fr_den;
> +
> +        if (avctx->framerate.num > 0 && avctx->framerate.den > 0)
> +            av_reduce(&fr_num, &fr_den,
> +                    avctx->framerate.num, avctx->framerate.den, 65535);
> +        else
> +            av_reduce(&fr_num, &fr_den,
> +                    avctx->time_base.den, avctx->time_base.num, 65535);

I think only framerate should apply here, not time_base.

I don't think the reduction does anything?  You're going to immediately round down on the division below anyway.  (And if that's a problem, maybe the guess_level function should take an AVRational instead of an int.)

> +        if (fr_den > 0)
> +            framerate = fr_num / fr_den;
> +        else
> +            framerate = fr_num;

As above, not sure the second branch is helpful.

>  
>          level = ff_h264_guess_level(sps->profile_idc,
>                                      avctx->bit_rate,
> +                                    framerate,
>                                      priv->mb_width  * 16,
>                                      priv->mb_height * 16,
>                                      priv->dpb_frames);
> 

Thanks,

- Mark
Decai Lin March 8, 2019, 6:07 a.m.
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of

> Mark Thompson

> Sent: 2019年3月7日 8:24

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH] lavc/h264_levels: add MaxMBPS

> checking and update fate test.

> 

> On 06/03/2019 08:21, Decai Lin wrote:

> > 1. add MaxMBPS checking for level idc setting to align with AVC spec

> >    AnnexA table A-1/A-6 level limits.

> > 2. update h264 level fate test.

> >

> > Signed-off-by: Decai Lin <decai.lin@intel.com>

> > ---

> >  libavcodec/h264_levels.c       |  5 +++++

> >  libavcodec/h264_levels.h       |  1 +

> >  libavcodec/h264_metadata_bsf.c | 14 +++++++++++++-

> > libavcodec/tests/h264_levels.c | 41

> > ++++++++++++++++++++++++++++++++++++++---

> >  libavcodec/vaapi_encode_h264.c | 13 +++++++++++++

> >  5 files changed, 70 insertions(+), 4 deletions(-)

> 

> Nice, this is a good idea.  Some comments below.

> 

> > diff --git a/libavcodec/h264_levels.c b/libavcodec/h264_levels.c index

> > 7a55116..e457dbe 100644

> > --- a/libavcodec/h264_levels.c

> > +++ b/libavcodec/h264_levels.c

> > @@ -89,11 +89,13 @@ const H264LevelDescriptor *ff_h264_get_level(int

> > level_idc,

> >

> >  const H264LevelDescriptor *ff_h264_guess_level(int profile_idc,

> >                                                 int64_t bitrate,

> > +                                               int framerate,

> >                                                 int width, int

> height,

> >                                                 int

> > max_dec_frame_buffering)  {

> >      int width_mbs  = (width  + 15) / 16;

> >      int height_mbs = (height + 15) / 16;

> > +    int64_t mb_processing_rate = (int64_t) (width_mbs * height_mbs *

> > + framerate);

> 

> This doesn't work - if it overflows you've already hit undefined behaviour in

> the calculation before the cast.

> 

> >      int no_cs3f = !(profile_idc == 66 ||

> >                      profile_idc == 77 ||

> >                      profile_idc == 88); @@ -108,6 +110,9 @@ const

> > H264LevelDescriptor *ff_h264_guess_level(int profile_idc,

> >          if (bitrate > (int64_t)level->max_br *

> h264_get_br_factor(profile_idc))

> >              continue;

> >

> > +        if (mb_processing_rate > (int64_t)level->max_mbps)

> > +            continue;

> > +

> >          if (width_mbs  * height_mbs > level->max_fs)

> >              continue;

> >          if (width_mbs  * width_mbs  > 8 * level->max_fs) diff --git

> > a/libavcodec/h264_levels.h b/libavcodec/h264_levels.h index

> > 4189fc6..0a0f410 100644

> > --- a/libavcodec/h264_levels.h

> > +++ b/libavcodec/h264_levels.h

> > @@ -46,6 +46,7 @@ const H264LevelDescriptor *ff_h264_get_level(int

> level_idc,

> >   */

> >  const H264LevelDescriptor *ff_h264_guess_level(int profile_idc,

> >                                                 int64_t bitrate,

> > +                                               int framerate,

> >                                                 int width, int

> height,

> >                                                 int

> > max_dec_frame_buffering);

> >

> > diff --git a/libavcodec/h264_metadata_bsf.c

> > b/libavcodec/h264_metadata_bsf.c index a17987a..61c2711 100644

> > --- a/libavcodec/h264_metadata_bsf.c

> > +++ b/libavcodec/h264_metadata_bsf.c

> > @@ -223,6 +223,7 @@ static int

> h264_metadata_update_sps(AVBSFContext *bsf,

> >              const H264LevelDescriptor *desc;

> >              int64_t bit_rate;

> >              int width, height, dpb_frames;

> > +            int framerate, fr_num, fr_den;

> >

> >              if (sps->vui.nal_hrd_parameters_present_flag) {

> >                  bit_rate =

> > (sps->vui.nal_hrd_parameters.bit_rate_value_minus1[0] + 1) * @@ -244,7

> +245,18 @@ static int h264_metadata_update_sps(AVBSFContext *bsf,

> >              height = 16 * (sps->pic_height_in_map_units_minus1 + 1)

> *

> >                  (2 - sps->frame_mbs_only_flag);

> >

> > -            desc = ff_h264_guess_level(sps->profile_idc, bit_rate,

> > +            if (ctx->tick_rate.num > 0 && ctx->tick_rate.den > 0)

> > +                av_reduce(&fr_num, &fr_den,

> > +                        ctx->tick_rate.num, ctx->tick_rate.den,

> 65535);

> > +            else

> > +                av_reduce(&fr_num, &fr_den,

> > +                        ctx->tick_rate.den, ctx->tick_rate.num,

> > + 65535);

> 

> Since the VUI fields are set above, I suggest using them instead here because

> that will include an existing rate in the stream.

> 

> Also, the tick rate is only directly usable as fieldrate if fixed_frame_rate_flag

> is set.

> 

> > +            if (fr_den > 0)

> > +                framerate = fr_num / fr_den;

> > +            else

> > +                framerate = fr_num;

> 

> I'm not sure what this is trying to do.  If the number isn't valid, the

> framerate should be passed as zero to avoid constraining anything.

> 

> > +

> > +            desc = ff_h264_guess_level(sps->profile_idc, bit_rate,

> > + framerate,

> >                                         width, height, dpb_frames);

> >              if (desc) {

> >                  level_idc = desc->level_idc; diff --git

> > a/libavcodec/tests/h264_levels.c b/libavcodec/tests/h264_levels.c

> > index 0e00f05..6176c0b 100644

> > --- a/libavcodec/tests/h264_levels.c

> > +++ b/libavcodec/tests/h264_levels.c

> > @@ -62,6 +62,31 @@ static const struct {  static const struct {

> >      int width;

> >      int height;

> > +    int framerate;

> > +    int level_idc;

> > +} test_framerate[] = {

> > +    // Some typical sizes and frame rates.

> > +    // (From H.264 table A-1 and table A-6)

> > +    {  176,  144,  15, 10 },

> > +    {  320,  240,  10, 11 },

> > +    {  352,  288,  30, 13 },

> > +    {  352,  576,  25, 21 },

> > +    {  640,  480,  33, 30 },

> > +    {  720,  576,  25, 30 },

> > +    { 1024,  768,  35, 31 },

> > +    { 1280,  720,  30, 31 },

> > +    { 1280, 1024,  42, 32 },

> > +    { 1920, 1088,  30, 40 },

> > +    { 2048, 1024,  30, 40 },

> > +    { 2048, 1088,  60, 42 },

> > +    { 3680, 1536,  26, 50 },

> > +    { 4096, 2048,  30, 51 },

> > +    { 4096, 2160,  60, 52 },

> 

> These are all just under the constraint - add a few which are just over the

> given numbers too.  E.g. 1280x720 at 31fps doesn't fit in level 3.1 so needs

> 3.2.

> 

> > +};

> > +

> > +static const struct {

> > +    int width;

> > +    int height;

> >      int dpb_size;

> >      int level_idc;

> >  } test_dpb[] = {

> > @@ -147,14 +172,23 @@ int main(void)

> >      } while (0)

> >

> >      for (i = 0; i < FF_ARRAY_ELEMS(test_sizes); i++) {

> > -        level = ff_h264_guess_level(0, 0, test_sizes[i].width,

> > +        level = ff_h264_guess_level(0, 0, 0, test_sizes[i].width,

> >                                      test_sizes[i].height, 0);

> >          CHECK(test_sizes[i].level_idc, "size %dx%d",

> >                test_sizes[i].width, test_sizes[i].height);

> >      }

> >

> > +    for (i = 0; i < FF_ARRAY_ELEMS(test_framerate); i++) {

> > +        level = ff_h264_guess_level(0, 0, test_framerate[i].framerate,

> > +                                    test_framerate[i].width,

> > +                                    test_framerate[i].height, 0);

> > +        CHECK(test_framerate[i].level_idc, "framerate %d, size %dx%d",

> > +              test_framerate[i].framerate, test_framerate[i].width,

> > +              test_framerate[i].height);

> > +    }

> > +

> >      for (i = 0; i < FF_ARRAY_ELEMS(test_dpb); i++) {

> > -        level = ff_h264_guess_level(0, 0, test_dpb[i].width,

> > +        level = ff_h264_guess_level(0, 0, 0, test_dpb[i].width,

> >                                      test_dpb[i].height,

> >                                      test_dpb[i].dpb_size);

> >          CHECK(test_dpb[i].level_idc, "size %dx%d dpb %d", @@ -164,7

> > +198,7 @@ int main(void)

> >

> >      for (i = 0; i < FF_ARRAY_ELEMS(test_bitrate); i++) {

> >          level = ff_h264_guess_level(test_bitrate[i].profile_idc,

> > -                                    test_bitrate[i].bitrate,

> > +                                    test_bitrate[i].bitrate, 0,

> >                                      0, 0, 0);

> >          CHECK(test_bitrate[i].level_idc, "bitrate %"PRId64" profile %d",

> >                test_bitrate[i].bitrate, test_bitrate[i].profile_idc);

> > @@ -173,6 +207,7 @@ int main(void)

> >      for (i = 0; i < FF_ARRAY_ELEMS(test_all); i++) {

> >          level = ff_h264_guess_level(test_all[i].profile_idc,

> >                                      test_all[i].bitrate,

> > +                                    0,

> >                                      test_all[i].width,

> >                                      test_all[i].height,

> >                                      test_all[i].dpb_frames); diff

> > --git a/libavcodec/vaapi_encode_h264.c

> > b/libavcodec/vaapi_encode_h264.c index 91be33f..7e7fd6a 100644

> > --- a/libavcodec/vaapi_encode_h264.c

> > +++ b/libavcodec/vaapi_encode_h264.c

> > @@ -329,9 +329,22 @@ static int

> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)

> >          sps->level_idc = avctx->level;

> >      } else {

> >          const H264LevelDescriptor *level;

> > +        int framerate, fr_num, fr_den;

> > +

> > +        if (avctx->framerate.num > 0 && avctx->framerate.den > 0)

> > +            av_reduce(&fr_num, &fr_den,

> > +                    avctx->framerate.num, avctx->framerate.den,

> 65535);

> > +        else

> > +            av_reduce(&fr_num, &fr_den,

> > +                    avctx->time_base.den, avctx->time_base.num,

> > + 65535);

> 

> I think only framerate should apply here, not time_base.

> 

> I don't think the reduction does anything?  You're going to immediately

> round down on the division below anyway.  (And if that's a problem, maybe

> the guess_level function should take an AVRational instead of an int.)

> 

> > +        if (fr_den > 0)

> > +            framerate = fr_num / fr_den;

> > +        else

> > +            framerate = fr_num;

> 

> As above, not sure the second branch is helpful.

> 

> >

> >          level = ff_h264_guess_level(sps->profile_idc,

> >                                      avctx->bit_rate,

> > +                                    framerate,

> >                                      priv->mb_width  * 16,

> >                                      priv->mb_height * 16,

> >                                      priv->dpb_frames);

> >

> 


Thanks Mark, will update based on your comments.

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Patch hide | download patch | download mbox

diff --git a/libavcodec/h264_levels.c b/libavcodec/h264_levels.c
index 7a55116..e457dbe 100644
--- a/libavcodec/h264_levels.c
+++ b/libavcodec/h264_levels.c
@@ -89,11 +89,13 @@  const H264LevelDescriptor *ff_h264_get_level(int level_idc,
 
 const H264LevelDescriptor *ff_h264_guess_level(int profile_idc,
                                                int64_t bitrate,
+                                               int framerate,
                                                int width, int height,
                                                int max_dec_frame_buffering)
 {
     int width_mbs  = (width  + 15) / 16;
     int height_mbs = (height + 15) / 16;
+    int64_t mb_processing_rate = (int64_t) (width_mbs * height_mbs * framerate);
     int no_cs3f = !(profile_idc == 66 ||
                     profile_idc == 77 ||
                     profile_idc == 88);
@@ -108,6 +110,9 @@  const H264LevelDescriptor *ff_h264_guess_level(int profile_idc,
         if (bitrate > (int64_t)level->max_br * h264_get_br_factor(profile_idc))
             continue;
 
+        if (mb_processing_rate > (int64_t)level->max_mbps)
+            continue;
+
         if (width_mbs  * height_mbs > level->max_fs)
             continue;
         if (width_mbs  * width_mbs  > 8 * level->max_fs)
diff --git a/libavcodec/h264_levels.h b/libavcodec/h264_levels.h
index 4189fc6..0a0f410 100644
--- a/libavcodec/h264_levels.h
+++ b/libavcodec/h264_levels.h
@@ -46,6 +46,7 @@  const H264LevelDescriptor *ff_h264_get_level(int level_idc,
  */
 const H264LevelDescriptor *ff_h264_guess_level(int profile_idc,
                                                int64_t bitrate,
+                                               int framerate,
                                                int width, int height,
                                                int max_dec_frame_buffering);
 
diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index a17987a..61c2711 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -223,6 +223,7 @@  static int h264_metadata_update_sps(AVBSFContext *bsf,
             const H264LevelDescriptor *desc;
             int64_t bit_rate;
             int width, height, dpb_frames;
+            int framerate, fr_num, fr_den;
 
             if (sps->vui.nal_hrd_parameters_present_flag) {
                 bit_rate = (sps->vui.nal_hrd_parameters.bit_rate_value_minus1[0] + 1) *
@@ -244,7 +245,18 @@  static int h264_metadata_update_sps(AVBSFContext *bsf,
             height = 16 * (sps->pic_height_in_map_units_minus1 + 1) *
                 (2 - sps->frame_mbs_only_flag);
 
-            desc = ff_h264_guess_level(sps->profile_idc, bit_rate,
+            if (ctx->tick_rate.num > 0 && ctx->tick_rate.den > 0)
+                av_reduce(&fr_num, &fr_den,
+                        ctx->tick_rate.num, ctx->tick_rate.den, 65535);
+            else
+                av_reduce(&fr_num, &fr_den,
+                        ctx->tick_rate.den, ctx->tick_rate.num, 65535);
+            if (fr_den > 0)
+                framerate = fr_num / fr_den;
+            else
+                framerate = fr_num;
+
+            desc = ff_h264_guess_level(sps->profile_idc, bit_rate, framerate,
                                        width, height, dpb_frames);
             if (desc) {
                 level_idc = desc->level_idc;
diff --git a/libavcodec/tests/h264_levels.c b/libavcodec/tests/h264_levels.c
index 0e00f05..6176c0b 100644
--- a/libavcodec/tests/h264_levels.c
+++ b/libavcodec/tests/h264_levels.c
@@ -62,6 +62,31 @@  static const struct {
 static const struct {
     int width;
     int height;
+    int framerate;
+    int level_idc;
+} test_framerate[] = {
+    // Some typical sizes and frame rates.
+    // (From H.264 table A-1 and table A-6)
+    {  176,  144,  15, 10 },
+    {  320,  240,  10, 11 },
+    {  352,  288,  30, 13 },
+    {  352,  576,  25, 21 },
+    {  640,  480,  33, 30 },
+    {  720,  576,  25, 30 },
+    { 1024,  768,  35, 31 },
+    { 1280,  720,  30, 31 },
+    { 1280, 1024,  42, 32 },
+    { 1920, 1088,  30, 40 },
+    { 2048, 1024,  30, 40 },
+    { 2048, 1088,  60, 42 },
+    { 3680, 1536,  26, 50 },
+    { 4096, 2048,  30, 51 },
+    { 4096, 2160,  60, 52 },
+};
+
+static const struct {
+    int width;
+    int height;
     int dpb_size;
     int level_idc;
 } test_dpb[] = {
@@ -147,14 +172,23 @@  int main(void)
     } while (0)
 
     for (i = 0; i < FF_ARRAY_ELEMS(test_sizes); i++) {
-        level = ff_h264_guess_level(0, 0, test_sizes[i].width,
+        level = ff_h264_guess_level(0, 0, 0, test_sizes[i].width,
                                     test_sizes[i].height, 0);
         CHECK(test_sizes[i].level_idc, "size %dx%d",
               test_sizes[i].width, test_sizes[i].height);
     }
 
+    for (i = 0; i < FF_ARRAY_ELEMS(test_framerate); i++) {
+        level = ff_h264_guess_level(0, 0, test_framerate[i].framerate,
+                                    test_framerate[i].width,
+                                    test_framerate[i].height, 0);
+        CHECK(test_framerate[i].level_idc, "framerate %d, size %dx%d",
+              test_framerate[i].framerate, test_framerate[i].width,
+              test_framerate[i].height);
+    }
+
     for (i = 0; i < FF_ARRAY_ELEMS(test_dpb); i++) {
-        level = ff_h264_guess_level(0, 0, test_dpb[i].width,
+        level = ff_h264_guess_level(0, 0, 0, test_dpb[i].width,
                                     test_dpb[i].height,
                                     test_dpb[i].dpb_size);
         CHECK(test_dpb[i].level_idc, "size %dx%d dpb %d",
@@ -164,7 +198,7 @@  int main(void)
 
     for (i = 0; i < FF_ARRAY_ELEMS(test_bitrate); i++) {
         level = ff_h264_guess_level(test_bitrate[i].profile_idc,
-                                    test_bitrate[i].bitrate,
+                                    test_bitrate[i].bitrate, 0,
                                     0, 0, 0);
         CHECK(test_bitrate[i].level_idc, "bitrate %"PRId64" profile %d",
               test_bitrate[i].bitrate, test_bitrate[i].profile_idc);
@@ -173,6 +207,7 @@  int main(void)
     for (i = 0; i < FF_ARRAY_ELEMS(test_all); i++) {
         level = ff_h264_guess_level(test_all[i].profile_idc,
                                     test_all[i].bitrate,
+                                    0,
                                     test_all[i].width,
                                     test_all[i].height,
                                     test_all[i].dpb_frames);
diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index 91be33f..7e7fd6a 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -329,9 +329,22 @@  static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
         sps->level_idc = avctx->level;
     } else {
         const H264LevelDescriptor *level;
+        int framerate, fr_num, fr_den;
+
+        if (avctx->framerate.num > 0 && avctx->framerate.den > 0)
+            av_reduce(&fr_num, &fr_den,
+                    avctx->framerate.num, avctx->framerate.den, 65535);
+        else
+            av_reduce(&fr_num, &fr_den,
+                    avctx->time_base.den, avctx->time_base.num, 65535);
+        if (fr_den > 0)
+            framerate = fr_num / fr_den;
+        else
+            framerate = fr_num;
 
         level = ff_h264_guess_level(sps->profile_idc,
                                     avctx->bit_rate,
+                                    framerate,
                                     priv->mb_width  * 16,
                                     priv->mb_height * 16,
                                     priv->dpb_frames);