| Message ID | 20170427131026.3793-2-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | 48b3117844177d8442bc9fa3ede1d31ce82ae6fc |
| Headers | show |
L'octidi 8 floréal, an CCXXV, Michael Niedermayer a écrit : > The code does use 16bit sized arrays later so larger deltas would not work > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/svq3.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c > index 3e35fd73d6..76a465b9c0 100644 > --- a/libavcodec/svq3.c > +++ b/libavcodec/svq3.c > @@ -551,7 +551,7 @@ static inline int svq3_mc_dir(SVQ3Context *s, int size, int mode, > dy = get_interleaved_se_golomb(&s->gb_slice); > dx = get_interleaved_se_golomb(&s->gb_slice); > > - if (dx == INVALID_VLC || dy == INVALID_VLC) { > + if (dx != (int16_t)dx || dy != (int16_t)dy) { Is this cast not an undefined behaviour if dx is not in the range? An explicit "& 0x7FFF" may be better. Or using uint16_t, I do not know the code. > av_log(s->avctx, AV_LOG_ERROR, "invalid MV vlc\n"); > return -1; > } Regards,
On Thu, Apr 27, 2017 at 03:22:53PM +0200, Nicolas George wrote: > L'octidi 8 floréal, an CCXXV, Michael Niedermayer a écrit : > > The code does use 16bit sized arrays later so larger deltas would not work > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/svq3.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c > > index 3e35fd73d6..76a465b9c0 100644 > > --- a/libavcodec/svq3.c > > +++ b/libavcodec/svq3.c > > @@ -551,7 +551,7 @@ static inline int svq3_mc_dir(SVQ3Context *s, int size, int mode, > > dy = get_interleaved_se_golomb(&s->gb_slice); > > dx = get_interleaved_se_golomb(&s->gb_slice); > > > > - if (dx == INVALID_VLC || dy == INVALID_VLC) { > > > + if (dx != (int16_t)dx || dy != (int16_t)dy) { > > Is this cast not an undefined behaviour if dx is not in the range? It is implementation defined (6.3.1.3.3 ISO/IEC 9899:TC3) We require Twos-complement style definition for signed integers > > An explicit "& 0x7FFF" may be better. Or using uint16_t, I do not know > the code. The value is signed, so these will not work without additional code [...]
Hi, On Thu, Apr 27, 2017 at 9:56 AM, Michael Niedermayer <michael@niedermayer.cc > wrote: > We require Twos-complement style definition for signed integers Do we? Where is that documented? https://www.google.com/search?q=twos-complement+ffmpeg+site:ffmpeg.org+-pipermail Ronald
L'octidi 8 floréal, an CCXXV, Ronald S. Bultje a écrit : > Do we? Where is that documented? > > https://www.google.com/search?q=twos-complement+ffmpeg+site:ffmpeg.org+-pipermail I think it is not documented anywhere but in the head of various developers, but I remember at least one other instance. And I think it is a very reasonable assumption, at least for certain parts of the code. Regards,
Hi, On Thu, Apr 27, 2017 at 10:26 AM, Nicolas George <george@nsup.org> wrote: > L'octidi 8 floréal, an CCXXV, Ronald S. Bultje a écrit : > > Do we? Where is that documented? > > > > https://www.google.com/search?q=twos-complement+ffmpeg+site: > ffmpeg.org+-pipermail > > I think it is not documented anywhere but in the head of various > developers, but I remember at least one other instance. > > And I think it is a very reasonable assumption, at least for certain > parts of the code. I agree, I've heard it before as well. I think it's good to document these things publicly so that there's no risk for partisan bickering. :) Ronald
diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c index 3e35fd73d6..76a465b9c0 100644 --- a/libavcodec/svq3.c +++ b/libavcodec/svq3.c @@ -551,7 +551,7 @@ static inline int svq3_mc_dir(SVQ3Context *s, int size, int mode, dy = get_interleaved_se_golomb(&s->gb_slice); dx = get_interleaved_se_golomb(&s->gb_slice); - if (dx == INVALID_VLC || dy == INVALID_VLC) { + if (dx != (int16_t)dx || dy != (int16_t)dy) { av_log(s->avctx, AV_LOG_ERROR, "invalid MV vlc\n"); return -1; }
The code does use 16bit sized arrays later so larger deltas would not work Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/svq3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)