[Libva] [RFC PATCH Libva-intel-driver 1/6] H264_Encoding: Parse the packed header data from user to fix the hacked code of HW skip bytes
Zhao, Yakui
yakui.zhao at intel.com
Tue May 20 23:57:00 PDT 2014
On Wed, 2014-05-21 at 00:26 -0600, Gwenole Beauchesne wrote:
> Hi Yakui,
>
> Thanks for working on this. Additional comments.
Thanks for your comment.
>
> 2014-05-21 7:51 GMT+02:00 Zhao, Yakui <yakui.zhao at intel.com>:
>
> > 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 <yakui.zhao at intel.com>
> > ---
> > 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.
[Yakui]: What you mentioned is an issue if the user add a lot of
leading_zero_8bits which is beyond the HW range(I also consider this
scenario). But at most cases it is a corner case. So I don't add a lot
of check/adjustment for this corner case.
Of course I can add some warning info so that the user-space app can get
it.
What do you think?
>
> 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.
[Yakui]: After this is added, it will have no issue that the returned
value is beyond the limited range of skip emul bytes.
But after adjusting the offset/filled size, this will be different with
the content passed from the user(for example: some specific symbols).
Maybe this is not what they expected. So I use the solution that simply
inserts the packed rawdata and don't skip the corresponding
leading_zero_bits.
>
> Another thing, also include MVCD. i.e. nal_unit_type == 21, IIRC.
[Yakui]: Really? It seems that this is one reserved type from the H264
spec.
>
> 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
> > Libva at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/libva
More information about the Libva
mailing list