[x264-devel] [PATCH] Fix float-cast-overflow in x264_ratecontrol_end function.

[Alexey Samsonov]

Sure, I don't mind changing it to int16_t. Let me know if you want me to
upload the new patch.

On Tue, Jan 26, 2016 at 9:54 AM, BugMaster <BugMaster at narod.ru> wrote:

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



-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x264-devel/attachments/20160126/7631d17b/attachment.html>