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

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

Details

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

Commit Message

Decai Lin March 8, 2019, 6:39 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       |  6 +++++
 libavcodec/h264_levels.h       |  1 +
 libavcodec/h264_metadata_bsf.c | 10 +++++++-
 libavcodec/tests/h264_levels.c | 58 +++++++++++++++++++++++++++++++++++++++---
 libavcodec/vaapi_encode_h264.c |  7 +++++
 5 files changed, 78 insertions(+), 4 deletions(-)

Comments

Decai Lin March 20, 2019, 2:32 a.m.
Any comments before applying the patch?

> -----Original Message-----

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

> Decai Lin

> Sent: 2019年3月8日 14:39

> To: ffmpeg-devel@ffmpeg.org

> Cc: Lin, Decai <decai.lin@intel.com>

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

> checking and update fate test.

> 

> 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       |  6 +++++

>  libavcodec/h264_levels.h       |  1 +

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

> libavcodec/tests/h264_levels.c | 58

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

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

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

> 

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

> 7a55116..7fdb61d 100644

> --- a/libavcodec/h264_levels.c

> +++ b/libavcodec/h264_levels.c

> @@ -89,6 +89,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)  { @@ -116,6 +117,11 @@ const

> H264LevelDescriptor *ff_h264_guess_level(int profile_idc,

>              continue;

> 

>          if (width_mbs && height_mbs) {

> +            if (framerate > level->max_mbps / (width_mbs *

> height_mbs))

> +                continue;

> +        }

> +

> +        if (width_mbs && height_mbs) {

>              int max_dpb_frames =

>                  FFMIN(level->max_dpb_mbs / (width_mbs *

> height_mbs), 16);

>              if (max_dec_frame_buffering > max_dpb_frames) 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..4ad9664 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;

> 

>              if (sps->vui.nal_hrd_parameters_present_flag) {

>                  bit_rate =

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

> +245,14 @@ 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 (sps->vui.timing_info_present_flag) {

> +                framerate = sps->vui.time_scale /

> sps->vui.num_units_in_tick;

> +            }

> +            else {

> +                framerate = 0;

> +            }

> +

> +            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..1f2c2eb 100644

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

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

> @@ -62,6 +62,48 @@ 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 },

> +    {  176,  144,  16, 11 },

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

> +    {  320,  240,  20, 12 },

> +    {  320,  240,  40, 21 },

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

> +    {  352,  288,  51, 22 },

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

> +    {  352,  576,  26, 30 },

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

> +    {  640,  480,  34, 31 },

> +    {  720,  480,  50, 31 },

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

> +    {  800,  600,  55, 31 },

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

> +    { 1024,  768,  70, 32 },

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

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

> +    { 1280,  960,  45, 32 },

> +    { 1280,  960,  46, 40 },

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

> +    { 1600, 1200,  32, 40 },

> +    { 1600, 1200,  33, 42 },

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

> +    { 1920, 1088,  55, 42 },

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

> +    { 2048, 1024,  62, 42 },

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

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

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

> +    { 4096, 2048,  59, 52 },

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

> +};

> +

> +static const struct {

> +    int width;

> +    int height;

>      int dpb_size;

>      int level_idc;

>  } test_dpb[] = {

> @@ -147,14 +189,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

> +215,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 +224,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..4cf99d7 100644

> --- a/libavcodec/vaapi_encode_h264.c

> +++ b/libavcodec/vaapi_encode_h264.c

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

> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)

>          sps->level_idc = avctx->level;

>      } else {

>          const H264LevelDescriptor *level;

> +        int framerate;

> +

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

> +            framerate = avctx->framerate.num / avctx->framerate.den;

> +        else

> +            framerate = 0;

> 

>          level = ff_h264_guess_level(sps->profile_idc,

>                                      avctx->bit_rate,

> +                                    framerate,

>                                      priv->mb_width  * 16,

>                                      priv->mb_height * 16,

>                                      priv->dpb_frames);

> --

> 1.8.3.1

> 

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Carl Eugen Hoyos March 20, 2019, 8:33 a.m.
2019-03-08 7:39 GMT+01:00, Decai Lin <decai.lin@intel.com>:
> 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       |  6 +++++
>  libavcodec/h264_levels.h       |  1 +
>  libavcodec/h264_metadata_bsf.c | 10 +++++++-
>  libavcodec/tests/h264_levels.c | 58
> +++++++++++++++++++++++++++++++++++++++---
>  libavcodec/vaapi_encode_h264.c |  7 +++++
>  5 files changed, 78 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/h264_levels.c b/libavcodec/h264_levels.c
> index 7a55116..7fdb61d 100644
> --- a/libavcodec/h264_levels.c
> +++ b/libavcodec/h264_levels.c
> @@ -89,6 +89,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)
>  {
> @@ -116,6 +117,11 @@ const H264LevelDescriptor *ff_h264_guess_level(int
> profile_idc,
>              continue;
>
>          if (width_mbs && height_mbs) {
> +            if (framerate > level->max_mbps / (width_mbs * height_mbs))
> +                continue;
> +        }
> +
> +        if (width_mbs && height_mbs) {

This looks very strange:
Do I miss something or can these blocks be merged /
the last two inserts be removed?

[...]

> @@ -244,7 +245,14 @@ 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 (sps->vui.timing_info_present_flag) {
> +                framerate = sps->vui.time_scale /
> sps->vui.num_units_in_tick;

> +            }
> +            else {

Please merge these lines.

Carl Eugen
Decai Lin March 20, 2019, 9:10 a.m.
> -----Original Message-----

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

> Carl Eugen Hoyos

> Sent: 2019年3月20日 16:33

> To: FFmpeg development discussions and patches

> <ffmpeg-devel@ffmpeg.org>

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

> checking and update fate test.

> 

> 2019-03-08 7:39 GMT+01:00, Decai Lin <decai.lin@intel.com>:

> > 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       |  6 +++++

> >  libavcodec/h264_levels.h       |  1 +

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

> > libavcodec/tests/h264_levels.c | 58

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

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

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

> >

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

> > 7a55116..7fdb61d 100644

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

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

> > @@ -89,6 +89,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)  { @@ -116,6 +117,11 @@ const

> > H264LevelDescriptor *ff_h264_guess_level(int profile_idc,

> >              continue;

> >

> >          if (width_mbs && height_mbs) {

> > +            if (framerate > level->max_mbps / (width_mbs *

> height_mbs))

> > +                continue;

> > +        }

> > +

> > +        if (width_mbs && height_mbs) {

> 

> This looks very strange:

> Do I miss something or can these blocks be merged / the last two inserts be

> removed?


OK, will merge them in one block. 

> [...]

> 

> > @@ -244,7 +245,14 @@ 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 (sps->vui.timing_info_present_flag) {

> > +                framerate = sps->vui.time_scale /

> > sps->vui.num_units_in_tick;

> 

> > +            }

> > +            else {

> 

> Please merge these lines.


Thanks, will merge these lines. 

> _______________________________________________

> 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..7fdb61d 100644
--- a/libavcodec/h264_levels.c
+++ b/libavcodec/h264_levels.c
@@ -89,6 +89,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)
 {
@@ -116,6 +117,11 @@  const H264LevelDescriptor *ff_h264_guess_level(int profile_idc,
             continue;
 
         if (width_mbs && height_mbs) {
+            if (framerate > level->max_mbps / (width_mbs * height_mbs))
+                continue;
+        }
+
+        if (width_mbs && height_mbs) {
             int max_dpb_frames =
                 FFMIN(level->max_dpb_mbs / (width_mbs * height_mbs), 16);
             if (max_dec_frame_buffering > max_dpb_frames)
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..4ad9664 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;
 
             if (sps->vui.nal_hrd_parameters_present_flag) {
                 bit_rate = (sps->vui.nal_hrd_parameters.bit_rate_value_minus1[0] + 1) *
@@ -244,7 +245,14 @@  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 (sps->vui.timing_info_present_flag) {
+                framerate = sps->vui.time_scale / sps->vui.num_units_in_tick;
+            }
+            else {
+                framerate = 0;
+            }
+
+            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..1f2c2eb 100644
--- a/libavcodec/tests/h264_levels.c
+++ b/libavcodec/tests/h264_levels.c
@@ -62,6 +62,48 @@  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 },
+    {  176,  144,  16, 11 },
+    {  320,  240,  10, 11 },
+    {  320,  240,  20, 12 },
+    {  320,  240,  40, 21 },
+    {  352,  288,  30, 13 },
+    {  352,  288,  51, 22 },
+    {  352,  576,  25, 21 },
+    {  352,  576,  26, 30 },
+    {  640,  480,  33, 30 },
+    {  640,  480,  34, 31 },
+    {  720,  480,  50, 31 },
+    {  720,  576,  25, 30 },
+    {  800,  600,  55, 31 },
+    { 1024,  768,  35, 31 },
+    { 1024,  768,  70, 32 },
+    { 1280,  720,  30, 31 },
+    { 1280,  720,  31, 32 },
+    { 1280,  960,  45, 32 },
+    { 1280,  960,  46, 40 },
+    { 1280, 1024,  42, 32 },
+    { 1600, 1200,  32, 40 },
+    { 1600, 1200,  33, 42 },
+    { 1920, 1088,  30, 40 },
+    { 1920, 1088,  55, 42 },
+    { 2048, 1024,  30, 40 },
+    { 2048, 1024,  62, 42 },
+    { 2048, 1088,  60, 42 },
+    { 3680, 1536,  26, 50 },
+    { 4096, 2048,  30, 51 },
+    { 4096, 2048,  59, 52 },
+    { 4096, 2160,  60, 52 },
+};
+
+static const struct {
+    int width;
+    int height;
     int dpb_size;
     int level_idc;
 } test_dpb[] = {
@@ -147,14 +189,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 +215,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 +224,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..4cf99d7 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -329,9 +329,16 @@  static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
         sps->level_idc = avctx->level;
     } else {
         const H264LevelDescriptor *level;
+        int framerate;
+
+        if (avctx->framerate.num > 0 && avctx->framerate.den > 0)
+            framerate = avctx->framerate.num / avctx->framerate.den;
+        else
+            framerate = 0;
 
         level = ff_h264_guess_level(sps->profile_idc,
                                     avctx->bit_rate,
+                                    framerate,
                                     priv->mb_width  * 16,
                                     priv->mb_height * 16,
                                     priv->dpb_frames);