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 "
>> + "supported with this VAAPI version.\n");