Re: [FFmpeg-devel] lavc/vaapi_encode_h264: respect "slices" option in h264 vaapi encoder

2017-08-10 Thread Jun Zhao 


On 2017/8/10 6:05, Mark Thompson wrote:
> On 02/08/17 06:56, Jun Zhao wrote:
>> From f9b42385faedd64dacf613785c393c7b025237c9 Mon Sep 17 00:00:00 2001
>> From: Jun Zhao 
>> Date: Tue, 1 Aug 2017 23:05:44 -0400
>> Subject: [PATCH V2 3/4] lavc/vaapi_encode_h264: respect "slices" option in
>>  h264 vaapi encoder
>>
>> Enable multi-slice support in AVC/H.264 vaapi encoder.
>>
>> Signed-off-by: Wang, Yi A 
>> Signed-off-by: Jun Zhao 
>> ---
>>  libavcodec/vaapi_encode_h264.c | 38 +-
>>  1 file changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
>> index f9fcd805a4..5dad6d10a5 100644
>> --- a/libavcodec/vaapi_encode_h264.c
>> +++ b/libavcodec/vaapi_encode_h264.c
>> @@ -141,6 +141,10 @@ typedef struct VAAPIEncodeH264Context {
>>  int mb_width;
>>  int mb_height;
>>  
>> +int slice_of_mbs;
>> +int slice_mod_mbs;
>> +int last_mb_index;
>> +
>>  int fixed_qp_idr;
>>  int fixed_qp_p;
>>  int fixed_qp_b;
>> @@ -957,6 +961,7 @@ static int 
>> vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
>>  VAEncPictureParameterBufferH264  *vpic = pic->codec_picture_params;
>>  VAAPIEncodeH264Context   *priv = ctx->priv_data;
>>  int i;
>> +int max_slices;
>>  
>>  if (pic->type == PICTURE_TYPE_IDR) {
>>  av_assert0(pic->display_order == pic->encode_order);
>> @@ -1002,7 +1007,22 @@ static int 
>> vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
>>  vpic->pic_fields.bits.idr_pic_flag = (pic->type == PICTURE_TYPE_IDR);
>>  vpic->pic_fields.bits.reference_pic_flag = (pic->type != 
>> PICTURE_TYPE_B);
>>  
>> -pic->nb_slices = 1;
>> +max_slices = 1;
>> +if (ctx->max_slices) {
>> +max_slices = FFMIN(priv->mb_height, ctx->max_slices);
> 
> Where is this coming from?  You don't enforce anything about rows below.

I use mb_height as the upper limit about MAX(slices) , but maybe I am wrong,
in va.h, I find the flag VA_ENC_SLICE_STRUCTURE_ARBITRARY_MACROBLOCKS with 
comment
"Driver supports an arbitrary number of rows(I guess rows is wrong 
comment in this, I think the right word is microblocks) per slice".

So I will drop this check in V3 patch. Tks.

> 
>> +if (avctx->slices > max_slices) {
>> +av_log(avctx, AV_LOG_WARNING, "The max slices number per frame "
>> +   "cannot more than %d.\n", max_slices);
> 
> So if you pass a value which is too high, you get some lower number instead?  
> It should probably hard fail here - I'm not sure what your use case is for 
> this option, but I imagine it being for conforming to the some specific 
> stream requirements (like bluray or whatever), so just ignoring the option in 
> that case is probably bad.

Agree.

> 
>> +} else {
>> +max_slices = avctx->slices;
>> +}
>> +}
>> +
>> +pic->nb_slices = max_slices;
>> +
>> +priv->slice_of_mbs  = (priv->mb_width * priv->mb_height) / 
>> pic->nb_slices;
>> +priv->slice_mod_mbs = (priv->mb_width * priv->mb_height) % 
>> pic->nb_slices;
>> +priv->last_mb_index = 0;
>>  
>>  return 0;
>>  }
>> @@ -1052,14 +1072,18 @@ static int 
>> vaapi_encode_h264_init_slice_params(AVCodecContext *avctx,
>>  av_assert0(0 && "invalid picture type");
>>  }
>>  
>> -// Only one slice per frame.
>> -vslice->macroblock_address = 0;
>> -vslice->num_macroblocks = priv->mb_width * priv->mb_height;
>> +vslice->macroblock_address = priv->last_mb_index;
>> +vslice->num_macroblocks = priv->slice_of_mbs + (priv->slice_mod_mbs > 0 
>> ? 1 : 0);
>> +if (priv->slice_mod_mbs > 0)
>> +priv->slice_mod_mbs--;
>> +priv->last_mb_index += vslice->num_macroblocks;
> 
> I'm still unconfortable about this method of distributing the macroblocks 
> equally to slices with rounding error at the top - slice boundaries are not 
> invisible at low bitrates.  I don't have anything better to suggest, though.

I don't have any idea to distribute MBs to slice, maybe we can follow x264 
style to forces rectangular slices ? I don't know.

> 
>>  
>>  vslice->macroblock_info = VA_INVALID_ID;
>>  
>>  vslice->pic_parameter_set_id = vpic->pic_parameter_set_id;
>> -vslice->idr_pic_id = priv->idr_pic_count++;
>> +vslice->idr_pic_id = priv->idr_pic_count;
>> +if (priv->last_mb_index == priv->mb_width * priv->mb_height)
>> +priv->idr_pic_count++;
>>  
>>  vslice->pic_order_cnt_lsb = (pic->display_order - priv->last_idr_frame) 
>> &
>>  ((1 << (4 + 
>> vseq->seq_fields.bits.log2_max_pic_order_cnt_lsb_minus4)) - 1);
>> @@ -1157,6 +1181,10 @@ static av_cold int 
>> vaapi_encode_h264_configure(AVCodecContext *avctx)
>>  #endif
>>  }
>>  
>> +if (!ctx->max_slices && avctx->slices > 0)
>> +av_log(avctx, AV_LOG_WARNING, "The encode slice option is not

Re: [FFmpeg-devel] lavc/vaapi_encode_h264: respect "slices" option in h264 vaapi encoder

2017-08-09 Thread Mark Thompson 
On 02/08/17 06:56, Jun Zhao wrote:
> From f9b42385faedd64dacf613785c393c7b025237c9 Mon Sep 17 00:00:00 2001
> From: Jun Zhao 
> Date: Tue, 1 Aug 2017 23:05:44 -0400
> Subject: [PATCH V2 3/4] lavc/vaapi_encode_h264: respect "slices" option in
>  h264 vaapi encoder
> 
> Enable multi-slice support in AVC/H.264 vaapi encoder.
> 
> Signed-off-by: Wang, Yi A 
> Signed-off-by: Jun Zhao 
> ---
>  libavcodec/vaapi_encode_h264.c | 38 +-
>  1 file changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> index f9fcd805a4..5dad6d10a5 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -141,6 +141,10 @@ typedef struct VAAPIEncodeH264Context {
>  int mb_width;
>  int mb_height;
>  
> +int slice_of_mbs;
> +int slice_mod_mbs;
> +int last_mb_index;
> +
>  int fixed_qp_idr;
>  int fixed_qp_p;
>  int fixed_qp_b;
> @@ -957,6 +961,7 @@ static int 
> vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
>  VAEncPictureParameterBufferH264  *vpic = pic->codec_picture_params;
>  VAAPIEncodeH264Context   *priv = ctx->priv_data;
>  int i;
> +int max_slices;
>  
>  if (pic->type == PICTURE_TYPE_IDR) {
>  av_assert0(pic->display_order == pic->encode_order);
> @@ -1002,7 +1007,22 @@ static int 
> vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
>  vpic->pic_fields.bits.idr_pic_flag = (pic->type == PICTURE_TYPE_IDR);
>  vpic->pic_fields.bits.reference_pic_flag = (pic->type != PICTURE_TYPE_B);
>  
> -pic->nb_slices = 1;
> +max_slices = 1;
> +if (ctx->max_slices) {
> +max_slices = FFMIN(priv->mb_height, ctx->max_slices);

Where is this coming from?  You don't enforce anything about rows below.

> +if (avctx->slices > max_slices) {
> +av_log(avctx, AV_LOG_WARNING, "The max slices number per frame "
> +   "cannot more than %d.\n", max_slices);

So if you pass a value which is too high, you get some lower number instead?  
It should probably hard fail here - I'm not sure what your use case is for this 
option, but I imagine it being for conforming to the some specific stream 
requirements (like bluray or whatever), so just ignoring the option in that 
case is probably bad.

> +} else {
> +max_slices = avctx->slices;
> +}
> +}
> +
> +pic->nb_slices = max_slices;
> +
> +priv->slice_of_mbs  = (priv->mb_width * priv->mb_height) / 
> pic->nb_slices;
> +priv->slice_mod_mbs = (priv->mb_width * priv->mb_height) % 
> pic->nb_slices;
> +priv->last_mb_index = 0;
>  
>  return 0;
>  }
> @@ -1052,14 +1072,18 @@ static int 
> vaapi_encode_h264_init_slice_params(AVCodecContext *avctx,
>  av_assert0(0 && "invalid picture type");
>  }
>  
> -// Only one slice per frame.
> -vslice->macroblock_address = 0;
> -vslice->num_macroblocks = priv->mb_width * priv->mb_height;
> +vslice->macroblock_address = priv->last_mb_index;
> +vslice->num_macroblocks = priv->slice_of_mbs + (priv->slice_mod_mbs > 0 
> ? 1 : 0);
> +if (priv->slice_mod_mbs > 0)
> +priv->slice_mod_mbs--;
> +priv->last_mb_index += vslice->num_macroblocks;

I'm still unconfortable about this method of distributing the macroblocks 
equally to slices with rounding error at the top - slice boundaries are not 
invisible at low bitrates.  I don't have anything better to suggest, though.

>  
>  vslice->macroblock_info = VA_INVALID_ID;
>  
>  vslice->pic_parameter_set_id = vpic->pic_parameter_set_id;
> -vslice->idr_pic_id = priv->idr_pic_count++;
> +vslice->idr_pic_id = priv->idr_pic_count;
> +if (priv->last_mb_index == priv->mb_width * priv->mb_height)
> +priv->idr_pic_count++;
>  
>  vslice->pic_order_cnt_lsb = (pic->display_order - priv->last_idr_frame) &
>  ((1 << (4 + 
> vseq->seq_fields.bits.log2_max_pic_order_cnt_lsb_minus4)) - 1);
> @@ -1157,6 +1181,10 @@ static av_cold int 
> vaapi_encode_h264_configure(AVCodecContext *avctx)
>  #endif
>  }
>  
> +if (!ctx->max_slices && avctx->slices > 0)
> +av_log(avctx, AV_LOG_WARNING, "The encode slice option is not "
> +   "supported with this VAAPI version.\n");

It's a driver constraint, not a VAAPI version one.

> +
>  return 0;
>  }
>  
> -- 
> 2.11.0
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] lavc/vaapi_encode_h264: respect "slices" option in h264 vaapi encoder

2017-08-01 Thread Jun Zhao 
From f9b42385faedd64dacf613785c393c7b025237c9 Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Tue, 1 Aug 2017 23:05:44 -0400
Subject: [PATCH V2 3/4] lavc/vaapi_encode_h264: respect "slices" option in
 h264 vaapi encoder

Enable multi-slice support in AVC/H.264 vaapi encoder.

Signed-off-by: Wang, Yi A 
Signed-off-by: Jun Zhao 
---
 libavcodec/vaapi_encode_h264.c | 38 +-
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
index f9fcd805a4..5dad6d10a5 100644
--- a/libavcodec/vaapi_encode_h264.c
+++ b/libavcodec/vaapi_encode_h264.c
@@ -141,6 +141,10 @@ typedef struct VAAPIEncodeH264Context {
 int mb_width;
 int mb_height;
 
+int slice_of_mbs;
+int slice_mod_mbs;
+int last_mb_index;
+
 int fixed_qp_idr;
 int fixed_qp_p;
 int fixed_qp_b;
@@ -957,6 +961,7 @@ static int 
vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
 VAEncPictureParameterBufferH264  *vpic = pic->codec_picture_params;
 VAAPIEncodeH264Context   *priv = ctx->priv_data;
 int i;
+int max_slices;
 
 if (pic->type == PICTURE_TYPE_IDR) {
 av_assert0(pic->display_order == pic->encode_order);
@@ -1002,7 +1007,22 @@ static int 
vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
 vpic->pic_fields.bits.idr_pic_flag = (pic->type == PICTURE_TYPE_IDR);
 vpic->pic_fields.bits.reference_pic_flag = (pic->type != PICTURE_TYPE_B);
 
-pic->nb_slices = 1;
+max_slices = 1;
+if (ctx->max_slices) {
+max_slices = FFMIN(priv->mb_height, ctx->max_slices);
+if (avctx->slices > max_slices) {
+av_log(avctx, AV_LOG_WARNING, "The max slices number per frame "
+   "cannot more than %d.\n", max_slices);
+} else {
+max_slices = avctx->slices;
+}
+}
+
+pic->nb_slices = max_slices;
+
+priv->slice_of_mbs  = (priv->mb_width * priv->mb_height) / pic->nb_slices;
+priv->slice_mod_mbs = (priv->mb_width * priv->mb_height) % pic->nb_slices;
+priv->last_mb_index = 0;
 
 return 0;
 }
@@ -1052,14 +1072,18 @@ static int 
vaapi_encode_h264_init_slice_params(AVCodecContext *avctx,
 av_assert0(0 && "invalid picture type");
 }
 
-// Only one slice per frame.
-vslice->macroblock_address = 0;
-vslice->num_macroblocks = priv->mb_width * priv->mb_height;
+vslice->macroblock_address = priv->last_mb_index;
+vslice->num_macroblocks = priv->slice_of_mbs + (priv->slice_mod_mbs > 0 ? 
1 : 0);
+if (priv->slice_mod_mbs > 0)
+priv->slice_mod_mbs--;
+priv->last_mb_index += vslice->num_macroblocks;
 
 vslice->macroblock_info = VA_INVALID_ID;
 
 vslice->pic_parameter_set_id = vpic->pic_parameter_set_id;
-vslice->idr_pic_id = priv->idr_pic_count++;
+vslice->idr_pic_id = priv->idr_pic_count;
+if (priv->last_mb_index == priv->mb_width * priv->mb_height)
+priv->idr_pic_count++;
 
 vslice->pic_order_cnt_lsb = (pic->display_order - priv->last_idr_frame) &
 ((1 << (4 + vseq->seq_fields.bits.log2_max_pic_order_cnt_lsb_minus4)) 
- 1);
@@ -1157,6 +1181,10 @@ static av_cold int 
vaapi_encode_h264_configure(AVCodecContext *avctx)
 #endif
 }
 
+if (!ctx->max_slices && avctx->slices > 0)
+av_log(avctx, AV_LOG_WARNING, "The encode slice option is not "
+   "supported with this VAAPI version.\n");
+
 return 0;
 }
 
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel