<div dir="ltr">Sure, I don't mind changing it to int16_t. Let me know if you want me to upload the new patch.<br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 26, 2016 at 9:54 AM, BugMaster <span dir="ltr"><<a href="mailto:BugMaster@narod.ru" target="_blank">BugMaster@narod.ru</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 Mon, 25 Jan 2016 16:05:25 -0800, Alexey Samsonov wrote:<br>
> According to C standard, it is undefined behavior to cast negative<br>
> floating point number to unsigned integer. Float-cast-overflow in<br>
> general is known to produce different results on different architectures.<br>
<br>
> Building x264 code with Clang and -fsanitize=float-cast-overflow<br>
> (<a href="http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#availablle-checks" rel="noreferrer" target="_blank">http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#availablle-checks</a>)<br>
> and running it on some real-life examples occasionally produces errors<br>
> of the form:<br>
<br>
> encoder/ratecontrol.c:1892: runtime error: value -5011.14 is<br>
> outside the range of representable values of type 'unsigned short'<br>
<br>
> Fix these errors by explicitly coding de-facto x86 behavior: casting<br>
> float to uint16_t through int.<br>
> ---<br>
>  encoder/ratecontrol.c | 2 +-<br>
>  1 file changed, 1 insertion(+), 1 deletion(-)<br>
<br>
> diff --git a/encoder/ratecontrol.c b/encoder/ratecontrol.c<br>
> index d8d06b3..a16755e 100644<br>
> --- a/encoder/ratecontrol.c<br>
> +++ b/encoder/ratecontrol.c<br>
> @@ -1889,7 +1889,7 @@ int x264_ratecontrol_end( x264_t *h, int bits, int *filler )<br>
>              uint8_t i_type = h->sh.i_type;<br>
>              /* Values are stored as big-endian FIX8.8 */<br>
>              for( int i = 0; i < h->mb.i_mb_count; i++ )<br>
> -                rc->mbtree.qp_buffer[0][i] = endian_fix16( h->fenc->f_qp_offset[i]*256.0 );<br>
> +                rc->mbtree.qp_buffer[0][i] = endian_fix16(<br>
> (int)(h->fenc->f_qp_offset[i]*256.0) );<br>
>              if( fwrite( &i_type, 1, 1, rc->p_mbtree_stat_file_out ) < 1 )<br>
>                  goto fail;<br>
>              if( fwrite( rc->mbtree.qp_buffer[0], sizeof(uint16_t),<br>
> h->mb.i_mb_count, rc->p_mbtree_stat_file_out ) < h->mb.i_mb_count )<br>
<br>
</div></div>Hi. Thanks for the patch. In general I am agree with this change but<br>
imho it should be cast to int16_t and not to int. int16_t should<br>
better explain the limits we expect in this cast (if for some reason<br>
this cast will be with saturation).<br>
<br>
_______________________________________________<br>
x264-devel mailing list<br>
<a href="mailto:x264-devel@videolan.org">x264-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x264-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/x264-devel</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div></div>
</div></div>