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

2019-03-20 Thread Lin, Decai


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Carl Eugen Hoyos
> Sent: 2019年3月20日 16:33
> To: FFmpeg development discussions and patches
> 
> 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 :
> > 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 
> > ---
> >  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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2019-03-20 Thread Carl Eugen Hoyos
2019-03-08 7:39 GMT+01:00, Decai Lin :
> 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 
> ---
>  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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2019-03-19 Thread Lin, Decai

Any comments before applying the patch?

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Decai Lin
> Sent: 2019年3月8日 14:39
> To: ffmpeg-devel@ffmpeg.org
> Cc: Lin, Decai 
> 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 
> ---
>  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