[Libva] [PATCH 2/3] Convert encoding misc param type to index
lizhong
zhong.li at intel.com
Wed May 14 23:40:12 PDT 2014
On 05/14/2014 04:55 PM, Gwenole Beauchesne wrote:
> Hi,
>
> This is close. When I meant "do the conversion once while
> parsing/accumulating", this is so that to further optimize the
> translation/usage process. In this patch, you still have to perform
> conversions afterwards, this is suboptimal.
>
> Here is the exact approach I had in mind:
>
> enum {
> /* This strictly follows the base VAEncMiscParameterTypeXXX */
> VAIntelEncMiscParameterTypeFrameRate,
> VAIntelEncMiscParameterTypeRateControl,
> ...
>
> /* This strictly follows the Intel specific extensions, in order */
> VAIntelEncMiscParameterTypeExtBase,
> VAIntelEncMiscParameterTypeFEIFrameControl =
> VAIntelEncMiscParameterTypeExtBase,
>
> VAIntelEncMiscParameterTypeCount
> };
>
> Your conversion function:
>
> switch (type) {
> case VAEncMiscParameterTypeFrameRate:
> case VAEncMiscParameterTypeRateControl:
> ...
> intel_type = VAIntelEncMiscParameterTypeFrameRate + (type -
> VAEncMiscParameterTypeFrameRate);
> break;
> case VAEncMiscParameterTypeFEIFrameControlIntel:
> ...
> intel_type = VAIntelEncMiscParameterTypeFEIFrameControl + (type -
> VAEncMiscParameterTypeFEIFrameControlIntel);
> break;
> default:
> /* error out | fix */
> /* XXX: or generate a "garbage" index that will hold them. Since we
> don't need them, it's fine that unused/unsupported buffers replace
> older while parsing */
> break;
> }
>
> So, once you filled up the internal buffer store in context, you can
> easily access them as misc_buffer[VAIntelEncMiscParameterTypeXXX]; No
> more conversion. And the initial conversion function will likely be
> compiler optimized too with 2 to 3 branches and a couple of subs.
>
> Thanks,
> Gwenole.
When I wanted to covert misc_param type, there were two ideas, one is
similar to you: Create an driver internal enumeration,
such as I965EncMiscParameterTypeXXX, then map VAEncMiscParameterXXX to
I965EncMiscParameterTypeXXX.
The second ideal is this patch. I prefer the second ideal because I
don't want to add internal data structure
(such as I965EncMiscParameterTypeXXX, or your
VAIntelEncMiscParameterTypeXXX ) in driver unless strong necessity,
though the first ideal is a little more efficient, but it can be ignored
in real test.
Anyway, thanks for your detail reply, it is a good reference.
> 2014-05-13 11:20 GMT+02:00 Zhong Li <zhong.li at intel.com>:
>> Add function va_enc_misc_param_type_to_idx(),
>> it is helpful to reduce misc_param of encode_state buffer size.
>>
>> Signed-off-by: Zhong Li <zhong.li at intel.com>
>> ---
>> src/gen6_mfc_common.c | 3 ++-
>> src/i965_drv_video.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
>> src/i965_drv_video.h | 3 +++
>> 3 files changed, 47 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/gen6_mfc_common.c b/src/gen6_mfc_common.c
>> index 33b9d55..faad699 100644
>> --- a/src/gen6_mfc_common.c
>> +++ b/src/gen6_mfc_common.c
>> @@ -134,7 +134,8 @@ static void intel_mfc_brc_init(struct encode_state *encode_state,
>> {
>> struct gen6_mfc_context *mfc_context = encoder_context->mfc_context;
>> VAEncSequenceParameterBufferH264 *pSequenceParameter = (VAEncSequenceParameterBufferH264 *)encode_state->seq_param_ext->buffer;
>> - VAEncMiscParameterBuffer* pMiscParamHRD = (VAEncMiscParameterBuffer*)encode_state->misc_param[VAEncMiscParameterTypeHRD]->buffer;
>> + int idx = va_enc_misc_param_type_to_idx(VAEncMiscParameterTypeHRD);
>> + VAEncMiscParameterBuffer* pMiscParamHRD = (VAEncMiscParameterBuffer*)encode_state->misc_param[idx]->buffer;
>> VAEncMiscParameterHRD* pParameterHRD = (VAEncMiscParameterHRD*)pMiscParamHRD->data;
>> double bitrate = pSequenceParameter->bits_per_second;
>> double framerate = (double)pSequenceParameter->time_scale /(2 * (double)pSequenceParameter->num_units_in_tick);
>> diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
>> index e505e4a..808c0d5 100755
>> --- a/src/i965_drv_video.c
>> +++ b/src/i965_drv_video.c
>> @@ -261,6 +261,43 @@ va_enc_packed_type_to_idx(int packed_type)
>> return idx;
>> }
>>
>> +#define I965_ENC_MISC_PARAM_TYPE_FEI_FRAME_CONTROL_INDEX 14
>> +
>> +int
>> +va_enc_misc_param_type_to_idx(int misc_param_type)
>> +{
>> + int idx = 0;
>> +
>> + switch (misc_param_type) {
>> + case VAEncMiscParameterTypeFrameRate:
>> + case VAEncMiscParameterTypeRateControl:
>> + case VAEncMiscParameterTypeMaxSliceSize:
>> + case VAEncMiscParameterTypeAIR:
>> + case VAEncMiscParameterTypeMaxFrameSize:
>> + case VAEncMiscParameterTypeHRD:
>> + case VAEncMiscParameterTypeQualityLevel:
>> + case VAEncMiscParameterTypeRIR:
>> + case VAEncMiscParameterTypeQuantization:
>> + case VAEncMiscParameterTypeSkipFrame:
>> + case VAEncMiscParameterTypeROI:
>> + case VAEncMiscParameterTypeCIR:
>> + idx = misc_param_type;
>> + break;
>> +
>> + case VAEncMiscParameterTypeFEIFrameControlIntel:
>> + idx = I965_ENC_MISC_PARAM_TYPE_FEI_FRAME_CONTROL_INDEX;
>> + break;
>> +
>> + default:
>> + /* Should not get here */
>> + ASSERT_RET(0, 0);
>> + break;
>> + }
>> +
>> + ASSERT_RET(idx < 16, 0);
>> + return idx;
>> +}
>> +
>> VAStatus
>> i965_QueryConfigProfiles(VADriverContextP ctx,
>> VAProfile *profile_list, /* out */
>> @@ -2171,17 +2208,19 @@ i965_encoder_render_misc_parameter_buffer(VADriverContextP ctx,
>> {
>> struct encode_state *encode = &obj_context->codec_state.encode;
>> VAEncMiscParameterBuffer *param = NULL;
>> + int type_index;
>>
>> ASSERT_RET(obj_buffer->buffer_store->bo == NULL, VA_STATUS_ERROR_INVALID_BUFFER);
>> ASSERT_RET(obj_buffer->buffer_store->buffer, VA_STATUS_ERROR_INVALID_BUFFER);
>>
>> param = (VAEncMiscParameterBuffer *)obj_buffer->buffer_store->buffer;
>> + type_index = va_enc_misc_param_type_to_idx(param->type);
>>
>> - if (param->type >= ARRAY_ELEMS(encode->misc_param))
>> + if (type_index >= ARRAY_ELEMS(encode->misc_param))
>> return VA_STATUS_ERROR_INVALID_PARAMETER;
>>
>> - i965_release_buffer_store(&encode->misc_param[param->type]);
>> - i965_reference_buffer_store(&encode->misc_param[param->type], obj_buffer->buffer_store);
>> + i965_release_buffer_store(&encode->misc_param[type_index]);
>> + i965_reference_buffer_store(&encode->misc_param[type_index], obj_buffer->buffer_store);
>>
>> return VA_STATUS_SUCCESS;
>> }
>> diff --git a/src/i965_drv_video.h b/src/i965_drv_video.h
>> index 7c2306c..d7e8eb2 100644
>> --- a/src/i965_drv_video.h
>> +++ b/src/i965_drv_video.h
>> @@ -391,6 +391,9 @@ i965_check_alloc_surface_bo(VADriverContextP ctx,
>> int
>> va_enc_packed_type_to_idx(int packed_type);
>>
>> +int
>> +va_enc_misc_param_type_to_idx(int misc_param_type);
>> +
>> /* reserve 2 byte for internal using */
>> #define CODEC_H264 0
>> #define CODEC_MPEG2 1
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Libva mailing list
>> Libva at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/libva
More information about the Libva
mailing list