Em Fri, 27 Sep 2019 16:34:11 +0200 Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx> escreveu: > This introduces support for HEVC/H.265 to the Cedrus VPU driver, with > both uni-directional and bi-directional prediction modes supported. > > Field-coded (interlaced) pictures, custom quantization matrices and > 10-bit output are not supported at this point. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx> > --- ... > + unsigned int ctb_size_luma = > + 1 << log2_max_luma_coding_block_size; Shifts like this is a little scary. "1" constant is signed. So, if log2_max_luma_coding_block_size is 31, the above logic has undefined behavior. Different archs and C compilers may handle it on different ways. > +#define VE_DEC_H265_LOW_ADDR_PRIMARY_CHROMA(a) \ > + (((a) << 24) & GENMASK(31, 24)) Same applies here and on other similar macros. You need to enforce (a) to be unsigned, as otherwise the behavior is undefined. Btw, this is a recurrent pattern on this file. I would define a macro, e. g. something like: #define MASK_BITS_AND_SHIFT(v, high, low) \ ((UL(v) << low) & GENMASK(high, low)) And use it for all similar patterns here. The best would be to include such macro at linux/bits.h, although some upstream discussion is required. So, for now, let's add it at this header file, but work upstream to have it merged there. Thanks, Mauro
