Hi Yakui, Thanks for working on this. Additional comments.
2014-05-21 7:51 GMT+02:00 Zhao, Yakui <[email protected]>: > When the packed header data from user is inserted into the coded clip, it uses > the hacked code to check the number of HW skip emulation bytes. This is wrong. > So fix it. > Of course if the packed header data is generated by the driver, it is > unnecessary to check it and it can still use the pre-defined number of HW > skip bytes. > > Signed-off-by: Zhao Yakui <[email protected]> > --- > src/gen6_mfc_common.c | 58 > ++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 55 insertions(+), 3 deletions(-) > > diff --git a/src/gen6_mfc_common.c b/src/gen6_mfc_common.c > index 33b9d55..f497e75 100644 > --- a/src/gen6_mfc_common.c > +++ b/src/gen6_mfc_common.c > @@ -405,6 +405,53 @@ void intel_mfc_brc_prepare(struct encode_state > *encode_state, > } > } > > +static int intel_avc_find_skipemulcnt(unsigned char *buf, int bits_length) > +{ > + int i, found; > + int leading_zero_cnt, byte_length, zero_byte; > + int nal_unit_type; > + int skip_cnt = 0; > + > +#define NAL_UNIT_TYPE_MASK 0x1f > + > + byte_length = ALIGN(bits_length, 32) >> 3; > + > + > + leading_zero_cnt = 0; > + found = 0; > + for(i = 0; i < byte_length - 4; i++) { > + if (((buf[i] == 0) && (buf[i + 1] == 0) && (buf[i + 2] == 1)) || > + ((buf[i] == 0) && (buf[i + 1] == 0) && (buf[i + 2] == 0) && > (buf[i + 3] == 1))) { > + found = 1; > + break; > + } > + leading_zero_cnt++; > + } > + if (!found) { > + /* warning message is complained. But anyway it will be inserted. */ > + WARN_ONCE("Invalid packed header data. " > + "Can't find the 000001 start_prefix code\n"); > + return 0; > + } > + i = leading_zero_cnt; > + > + zero_byte = 0; > + if (!((buf[i] == 0) && (buf[i + 1] == 0) && (buf[i + 2] == 1))) > + zero_byte = 1; > + > + skip_cnt = leading_zero_cnt + zero_byte + 3; > + > + /* the unit header byte is accounted */ > + nal_unit_type = (buf[skip_cnt]) & NAL_UNIT_TYPE_MASK; > + skip_cnt += 1; > + > + if (nal_unit_type == 14 || nal_unit_type == 20) { > + /* more unit header bytes are accounted for MVC/SVC */ > + skip_cnt += 3; > + } > + return skip_cnt; > +} > + > void intel_mfc_avc_pipeline_header_programing(VADriverContextP ctx, > struct encode_state > *encode_state, > struct intel_encoder_context > *encoder_context, Well, actually, what I meant was: 1. Skip the leading zeros. Really, i.e. even for the part submitted to the HW. 2. Start counting bytes to skip from this new offset. Reason: the HW supports a limited range of Skip emul bytes. So, if you skip the leading zeros, you can always fulfill this condition. Otherwise, you risk an overflow. Since it is a VA-API requirement for packed headers to contain the start code prefix, you could split that up into: 1. Search for the start code prefix (sc_offset) ; 2. Determine the nalUnitHeaderBytes count. Based on (1), you can safely make the sc_offset the new origin of the buffer to submit + reduce the programmed size accordingly. And (1) could also be shared with other codecs. Another thing, also include MVCD. i.e. nal_unit_type == 21, IIRC. Thanks, Gwenole. > @@ -413,6 +460,7 @@ void > intel_mfc_avc_pipeline_header_programing(VADriverContextP ctx, > struct gen6_mfc_context *mfc_context = encoder_context->mfc_context; > int idx = va_enc_packed_type_to_idx(VAEncPackedHeaderH264_SPS); > unsigned int rate_control_mode = encoder_context->rate_control_mode; > + unsigned int skip_emul_byte_cnt; > > if (encode_state->packed_header_data[idx]) { > VAEncPackedHeaderParameterBuffer *param = NULL; > @@ -423,12 +471,13 @@ void > intel_mfc_avc_pipeline_header_programing(VADriverContextP ctx, > param = (VAEncPackedHeaderParameterBuffer > *)encode_state->packed_header_param[idx]->buffer; > length_in_bits = param->bit_length; > > + skip_emul_byte_cnt = intel_avc_find_skipemulcnt((unsigned char > *)header_data, length_in_bits); > mfc_context->insert_object(ctx, > encoder_context, > header_data, > ALIGN(length_in_bits, 32) >> 5, > length_in_bits & 0x1f, > - 5, /* FIXME: check it */ > + skip_emul_byte_cnt, > 0, > 0, > !param->has_emulation_bytes, > @@ -446,12 +495,14 @@ void > intel_mfc_avc_pipeline_header_programing(VADriverContextP ctx, > param = (VAEncPackedHeaderParameterBuffer > *)encode_state->packed_header_param[idx]->buffer; > length_in_bits = param->bit_length; > > + skip_emul_byte_cnt = intel_avc_find_skipemulcnt((unsigned char > *)header_data, length_in_bits); > + > mfc_context->insert_object(ctx, > encoder_context, > header_data, > ALIGN(length_in_bits, 32) >> 5, > length_in_bits & 0x1f, > - 5, /* FIXME: check it */ > + skip_emul_byte_cnt, > 0, > 0, > !param->has_emulation_bytes, > @@ -469,12 +520,13 @@ void > intel_mfc_avc_pipeline_header_programing(VADriverContextP ctx, > param = (VAEncPackedHeaderParameterBuffer > *)encode_state->packed_header_param[idx]->buffer; > length_in_bits = param->bit_length; > > + skip_emul_byte_cnt = intel_avc_find_skipemulcnt((unsigned char > *)header_data, length_in_bits); > mfc_context->insert_object(ctx, > encoder_context, > header_data, > ALIGN(length_in_bits, 32) >> 5, > length_in_bits & 0x1f, > - 5, /* FIXME: check it */ > + skip_emul_byte_cnt, > 0, > 0, > !param->has_emulation_bytes, > -- > 1.7.12-rc1 > > _______________________________________________ > Libva mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/libva _______________________________________________ Libva mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libva
