[Mesa-dev] [PATCH 01/12] vl: add parameters for VAAPI encode

[Zhang, Boyuan]

Hi Emil,


Thanks for the suggestion. I added brief message to each of the patch to explain what the patch does. Please see the new patch set I just submitted.



Hi Christian,


The un-used ref_pic related definitions are removed from this patch.


For the concern of is_idr flag , I checked the behavior of Vaapi and the codes again. Vaapi treats both "idr-iframe" and "non-idr-iframe" same as i-frame in picture type, and it uses a separate flag to tell driver whether this iframe is idr or not. So from only the picture type, we can't tell whether it's idr or not. Since VCE needs this information, so I think we still need to add this flag.


Regards,

Boyuan

________________________________
From: Christian König <deathsimple at vodafone.de>
Sent: July 1, 2016 8:21 AM
To: Zhang, Boyuan; mesa-dev at lists.freedesktop.org
Subject: Re: [PATCH 01/12] vl: add parameters for VAAPI encode

Hi Boyuan,

as Emil wrote as well try to add some commit messages to the set. For
this patch something like the following should do it:

Allow to specify more parameters in the encoding interface which where
previously just hardcoded in the encoder.

Additional to that we need to reorder the patches a bit. First the
interface changes, then the OMX changes to fill in the previously
hardcoded values, then the radeon backend changes and then last the
VA-API changes to use the new interface.

Additional to that a few notes below.

Am 30.06.2016 um 20:30 schrieb Boyuan Zhang:
> Signed-off-by: Boyuan Zhang <boyuan.zhang at amd.com>
> ---
>   src/gallium/include/pipe/p_video_state.h | 36 ++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
>
> diff --git a/src/gallium/include/pipe/p_video_state.h b/src/gallium/include/pipe/p_video_state.h
> index d353be6..9cd489b 100644
> --- a/src/gallium/include/pipe/p_video_state.h
> +++ b/src/gallium/include/pipe/p_video_state.h
> @@ -352,9 +352,29 @@ struct pipe_h264_enc_rate_control
>      unsigned frame_rate_num;
>      unsigned frame_rate_den;
>      unsigned vbv_buffer_size;
> +   unsigned vbv_buf_lv;
>      unsigned target_bits_picture;
>      unsigned peak_bits_picture_integer;
>      unsigned peak_bits_picture_fraction;
> +   unsigned fill_data_enable;
> +   unsigned enforce_hrd;
> +};
> +
> +struct pipe_h264_enc_motion_estimation
> +{
> +   unsigned motion_est_quarter_pixel;
> +   unsigned enc_disable_sub_mode;
> +   unsigned lsmvert;
> +   unsigned enc_en_ime_overw_dis_subm;
> +   unsigned enc_ime_overw_dis_subm_no;
> +   unsigned enc_ime2_search_range_x;
> +   unsigned enc_ime2_search_range_y;
> +};
> +
> +struct pipe_h264_enc_pic_control
> +{
> +   unsigned enc_cabac_enable;
> +   unsigned enc_constraint_set_flags;
>   };
>
>   struct pipe_h264_enc_picture_desc
> @@ -363,17 +383,33 @@ struct pipe_h264_enc_picture_desc
>
>      struct pipe_h264_enc_rate_control rate_ctrl;
>
> +   struct pipe_h264_enc_motion_estimation motion_est;
> +   struct pipe_h264_enc_pic_control pic_ctrl;
> +
>      unsigned quant_i_frames;
>      unsigned quant_p_frames;
>      unsigned quant_b_frames;
>
>      enum pipe_h264_enc_picture_type picture_type;
>      unsigned frame_num;
> +   unsigned frame_num_cnt;
> +   unsigned p_remain;
> +   unsigned i_remain;
> +   unsigned idr_pic_id;
> +   unsigned gop_cnt;
>      unsigned pic_order_cnt;
>      unsigned ref_idx_l0;
>      unsigned ref_idx_l1;
> +   unsigned gop_size;
> +   unsigned ref_pic_mode;
>
>      bool not_referenced;
> +   bool is_idr;

Why can't this be inferred from the encoded picture type?

> +   bool has_ref_pic_list;
> +   bool enable_vui;
> +   unsigned int ref_pic_list_0[32];
> +   unsigned int ref_pic_list_1[32];
> +   unsigned int frame_idx[32];

I thought we wanted to drop the ref_pic_list handling for now. If that
is still the case please drop those fields here as well.

Regards,
Christian.

>   };
>
>   struct pipe_h265_sps

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160714/0fd30e94/attachment-0001.html>