<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 17, 2017 at 6:48 PM, Xiang, Haihao <span dir="ltr"><<a href="mailto:haihao.xiang@intel.com" target="_blank">haihao.xiang@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, 2017-01-17 at 13:15 -0800, Sean V Kelley wrote:<br>
> From: Kuang-che Wu <<a href="mailto:kcwu@chromium.org">kcwu@chromium.org</a>><br>
><br>
> shift uint32_t by 32 bits is undefined behavior.<br>
><br>
> For this particular case: when invoke avc_bitstream_put_ui() with 32<br>
> bits value at byte position of multiple of 4, existing 32 bits garbage<br>
> data in the buffer may be retained instead of cleared. The result is,<br>
> the position of NALU start code (0x00000001) looks like overwritten by<br>
> garbage value.<br>
><br>
> Patch has been tested and used upstream:<br>
> <a href="https://chromium-review.googlesource.com/#/c/410541/" rel="noreferrer" target="_blank">https://chromium-review.<wbr>googlesource.com/#/c/410541/</a><br>
><br>
> Signed-off-by: Kuang-che Wu <<a href="mailto:kcwu@chromium.org">kcwu@chromium.org</a>><br>
> Signed-off-by: Sean V Kelley <<a href="mailto:seanvk@posteo.de">seanvk@posteo.de</a>><br>
> ---<br>
>  src/i965_encoder_utils.c | 6 +++++-<br>
>  1 file changed, 5 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/src/i965_encoder_utils.c b/src/i965_encoder_utils.c<br>
> index ac58cd1a..e061d071 100644<br>
> --- a/src/i965_encoder_utils.c<br>
> +++ b/src/i965_encoder_utils.c<br>
> @@ -134,7 +134,11 @@ avc_bitstream_put_ui(avc_<wbr>bitstream *bs, unsigned int val, int size_in_bits)<br>
>          bs->buffer[pos] = (bs->buffer[pos] << size_in_bits | val);<br>
>      } else {<br>
>          size_in_bits -= bit_left;<br>
> -        bs->buffer[pos] = (bs->buffer[pos] << bit_left) | (val >> size_in_bits);<br>
> +        if (bit_left == 32) {<br>
> +            bs->buffer[pos] = (val >> size_in_bits);<br>
<br>
</div></div>The input value of size_in_bits should be less than or equal to 32, so now the<br>
value of size_in_bits is 0 for this case, I think  "bs->buffer[pos] = val" is<br>
more clear.<br></blockquote><div><br></div><div>That's a reasonable change.  Thanks Haihao.  I'll update the patch.</div><div><br>Sean</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="im HOEnZb"><br>
> +        } else {<br>
> +            bs->buffer[pos] = (bs->buffer[pos] << bit_left) | (val >> size_in_bits);<br>
> +        }<br>
>          bs->buffer[pos] = swap32(bs->buffer[pos]);<br>
>  <br>
>          if (pos + 1 == bs->max_size_in_dword) {<br>
</span><div class="HOEnZb"><div class="h5">______________________________<wbr>_________________<br>
Libva mailing list<br>
<a href="mailto:Libva@lists.freedesktop.org">Libva@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/libva" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/libva</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Sean V. Kelley <<a href="mailto:sean.v.kelley@intel.com" target="_blank">sean.v.kelley@intel.com</a>><br>Open Source Technology Center / SSG<br>Intel Corp.<br></div>
</div></div>