diff mbox

[FFmpeg-devel,v2,28/36] vaapi_encode_h264: Set level based on stream if not set by user

Message ID 20180607234331.32139-29-sw@jkqxz.net
State Superseded
Headers show

Commit Message

Mark Thompson June 7, 2018, 11:43 p.m. UTC 
---
 libavcodec/vaapi_encode_h264.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

Comments

Xiang, Haihao June 15, 2018, 2:24 a.m. UTC | #1 
On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
> ---

>  libavcodec/vaapi_encode_h264.c | 34 ++++++++++++++++++++++++++++++----

>  1 file changed, 30 insertions(+), 4 deletions(-)

> 

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

> index 82166d4457..4034053dc0 100644

> --- a/libavcodec/vaapi_encode_h264.c

> +++ b/libavcodec/vaapi_encode_h264.c

> @@ -30,6 +30,7 @@

>  #include "cbs.h"

>  #include "cbs_h264.h"

>  #include "h264.h"

> +#include "h264_levels.h"

>  #include "h264_sei.h"

>  #include "internal.h"

>  #include "vaapi_encode.h"

> @@ -294,6 +295,7 @@ static int

> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)

>      H264RawPPS                        *pps = &priv->raw_pps;

>      VAEncSequenceParameterBufferH264 *vseq = ctx->codec_sequence_params;

>      VAEncPictureParameterBufferH264  *vpic = ctx->codec_picture_params;

> +    int dpb_frames;

>  

>      memset(&priv->current_access_unit, 0,

>             sizeof(priv->current_access_unit));

> @@ -319,7 +321,32 @@ static int

> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)

>          sps->constraint_set5_flag = ctx->b_per_p == 0;

>      }

>  

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

> +    if (ctx->gop_size == 1)

> +        dpb_frames = 0;

> +    else

> +        dpb_frames = 1 + (ctx->b_per_p > 0);

> +

> +    if (avctx->level != FF_LEVEL_UNKNOWN) {

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


Is avctx->level always legal? if not, I think some checks on avctx->level should
be added.

> +    } else {

> +        const H264LevelDescriptor *level;

> +

> +        level = ff_h264_guess_level(sps->profile_idc,

> +                                    avctx->bit_rate,

> +                                    priv->mb_width  * 16,

> +                                    priv->mb_height * 16,

> +                                    dpb_frames);

> +        if (level) {

> +            av_log(avctx, AV_LOG_VERBOSE, "Using level %s.\n", level->name);

> +            if (level->constraint_set3_flag)

> +                sps->constraint_set3_flag = 1;

> +            sps->level_idc = level->level_idc;

> +        } else {

> +            av_log(avctx, AV_LOG_WARNING, "Stream will not conform "

> +                   "to any level: using level 6.2.\n");

> +            sps->level_idc = 62;

> +        }

> +    }

>  

>      sps->seq_parameter_set_id = 0;

>      sps->chroma_format_idc    = 1;

> @@ -329,8 +356,7 @@ static int

> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)

>      sps->log2_max_pic_order_cnt_lsb_minus4 =

>          av_clip(av_log2(ctx->b_per_p + 1) - 2, 0, 12);

>  

> -    sps->max_num_ref_frames =

> -        ctx->gop_size == 1 ? 0 : 1 + (ctx->b_per_p > 0);

> +    sps->max_num_ref_frames = dpb_frames;

>  

>      sps->pic_width_in_mbs_minus1        = priv->mb_width  - 1;

>      sps->pic_height_in_map_units_minus1 = priv->mb_height - 1;

> @@ -1005,7 +1031,7 @@ static const AVOption vaapi_encode_h264_options[] = {

>  

>      { "level", "Set level (level_idc)",

>        OFFSET(level), AV_OPT_TYPE_INT,

> -      { .i64 = 51 }, 0x00, 0xff, FLAGS, "level" },

> +      { .i64 = FF_LEVEL_UNKNOWN }, FF_LEVEL_UNKNOWN, 0xff, FLAGS, "level" },

>  

>  #define LEVEL(name, value) name, NULL, 0, AV_OPT_TYPE_CONST, \

>        { .i64 = value }, 0, 0, FLAGS, "level"
Mark Thompson June 17, 2018, 2:17 p.m. UTC | #2 
On 15/06/18 03:24, Xiang, Haihao wrote:
> On Fri, 2018-06-08 at 00:43 +0100, Mark Thompson wrote:
>> ---
>>  libavcodec/vaapi_encode_h264.c | 34 ++++++++++++++++++++++++++++++----
>>  1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
>> index 82166d4457..4034053dc0 100644
>> --- a/libavcodec/vaapi_encode_h264.c
>> +++ b/libavcodec/vaapi_encode_h264.c
>> @@ -30,6 +30,7 @@
>>  #include "cbs.h"
>>  #include "cbs_h264.h"
>>  #include "h264.h"
>> +#include "h264_levels.h"
>>  #include "h264_sei.h"
>>  #include "internal.h"
>>  #include "vaapi_encode.h"
>> @@ -294,6 +295,7 @@ static int
>> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>>      H264RawPPS                        *pps = &priv->raw_pps;
>>      VAEncSequenceParameterBufferH264 *vseq = ctx->codec_sequence_params;
>>      VAEncPictureParameterBufferH264  *vpic = ctx->codec_picture_params;
>> +    int dpb_frames;
>>  
>>      memset(&priv->current_access_unit, 0,
>>             sizeof(priv->current_access_unit));
>> @@ -319,7 +321,32 @@ static int
>> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>>          sps->constraint_set5_flag = ctx->b_per_p == 0;
>>      }
>>  
>> -    sps->level_idc = avctx->level;
>> +    if (ctx->gop_size == 1)
>> +        dpb_frames = 0;
>> +    else
>> +        dpb_frames = 1 + (ctx->b_per_p > 0);
>> +
>> +    if (avctx->level != FF_LEVEL_UNKNOWN) {
>> +        sps->level_idc = avctx->level;
> 
> Is avctx->level always legal? if not, I think some checks on avctx->level should
> be added.

I'll add a check that it fits in the 8-bit field.  I don't think it should be checked any more than that when the user has specified an explicit value.

>> +    } else {
>> +        const H264LevelDescriptor *level;
>> +
>> +        level = ff_h264_guess_level(sps->profile_idc,
>> +                                    avctx->bit_rate,
>> +                                    priv->mb_width  * 16,
>> +                                    priv->mb_height * 16,
>> +                                    dpb_frames);
>> +        if (level) {
>> +            av_log(avctx, AV_LOG_VERBOSE, "Using level %s.\n", level->name);
>> +            if (level->constraint_set3_flag)
>> +                sps->constraint_set3_flag = 1;
>> +            sps->level_idc = level->level_idc;
>> +        } else {
>> +            av_log(avctx, AV_LOG_WARNING, "Stream will not conform "
>> +                   "to any level: using level 6.2.\n");
>> +            sps->level_idc = 62;
>> +        }
>> +    }
>>  
>>      sps->seq_parameter_set_id = 0;
>>      sps->chroma_format_idc    = 1;
>> @@ -329,8 +356,7 @@ static int
>> vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>>      sps->log2_max_pic_order_cnt_lsb_minus4 =
>>          av_clip(av_log2(ctx->b_per_p + 1) - 2, 0, 12);
>>  
>> -    sps->max_num_ref_frames =
>> -        ctx->gop_size == 1 ? 0 : 1 + (ctx->b_per_p > 0);
>> +    sps->max_num_ref_frames = dpb_frames;
>>  
>>      sps->pic_width_in_mbs_minus1        = priv->mb_width  - 1;
>>      sps->pic_height_in_map_units_minus1 = priv->mb_height - 1;
>> @@ -1005,7 +1031,7 @@ static const AVOption vaapi_encode_h264_options[] = {
>>  
>>      { "level", "Set level (level_idc)",
>>        OFFSET(level), AV_OPT_TYPE_INT,
>> -      { .i64 = 51 }, 0x00, 0xff, FLAGS, "level" },
>> +      { .i64 = FF_LEVEL_UNKNOWN }, FF_LEVEL_UNKNOWN, 0xff, FLAGS, "level" },
>>  
>>  #define LEVEL(name, value) name, NULL, 0, AV_OPT_TYPE_CONST, \
>>        { .i64 = value }, 0, 0, FLAGS, "level"

Thanks,

- Mark
diff mbox

Patch

diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index 82166d4457..4034053dc0 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -30,6 +30,7 @@ 
 #include "cbs.h"
 #include "cbs_h264.h"
 #include "h264.h"
+#include "h264_levels.h"
 #include "h264_sei.h"
 #include "internal.h"
 #include "vaapi_encode.h"
@@ -294,6 +295,7 @@  static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
     H264RawPPS                        *pps = &priv->raw_pps;
     VAEncSequenceParameterBufferH264 *vseq = ctx->codec_sequence_params;
     VAEncPictureParameterBufferH264  *vpic = ctx->codec_picture_params;
+    int dpb_frames;
 
     memset(&priv->current_access_unit, 0,
            sizeof(priv->current_access_unit));
@@ -319,7 +321,32 @@  static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
         sps->constraint_set5_flag = ctx->b_per_p == 0;
     }
 
-    sps->level_idc = avctx->level;
+    if (ctx->gop_size == 1)
+        dpb_frames = 0;
+    else
+        dpb_frames = 1 + (ctx->b_per_p > 0);
+
+    if (avctx->level != FF_LEVEL_UNKNOWN) {
+        sps->level_idc = avctx->level;
+    } else {
+        const H264LevelDescriptor *level;
+
+        level = ff_h264_guess_level(sps->profile_idc,
+                                    avctx->bit_rate,
+                                    priv->mb_width  * 16,
+                                    priv->mb_height * 16,
+                                    dpb_frames);
+        if (level) {
+            av_log(avctx, AV_LOG_VERBOSE, "Using level %s.\n", level->name);
+            if (level->constraint_set3_flag)
+                sps->constraint_set3_flag = 1;
+            sps->level_idc = level->level_idc;
+        } else {
+            av_log(avctx, AV_LOG_WARNING, "Stream will not conform "
+                   "to any level: using level 6.2.\n");
+            sps->level_idc = 62;
+        }
+    }
 
     sps->seq_parameter_set_id = 0;
     sps->chroma_format_idc    = 1;
@@ -329,8 +356,7 @@  static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
     sps->log2_max_pic_order_cnt_lsb_minus4 =
         av_clip(av_log2(ctx->b_per_p + 1) - 2, 0, 12);
 
-    sps->max_num_ref_frames =
-        ctx->gop_size == 1 ? 0 : 1 + (ctx->b_per_p > 0);
+    sps->max_num_ref_frames = dpb_frames;
 
     sps->pic_width_in_mbs_minus1        = priv->mb_width  - 1;
     sps->pic_height_in_map_units_minus1 = priv->mb_height - 1;
@@ -1005,7 +1031,7 @@  static const AVOption vaapi_encode_h264_options[] = {
 
     { "level", "Set level (level_idc)",
       OFFSET(level), AV_OPT_TYPE_INT,
-      { .i64 = 51 }, 0x00, 0xff, FLAGS, "level" },
+      { .i64 = FF_LEVEL_UNKNOWN }, FF_LEVEL_UNKNOWN, 0xff, FLAGS, "level" },
 
 #define LEVEL(name, value) name, NULL, 0, AV_OPT_TYPE_CONST, \
       { .i64 = value }, 0, 0, FLAGS, "level"