Hi, 2014-08-24 12:13 GMT+02:00 Carl Eugen Hoyos <ceho...@ag.or.at>: [...] > - if( avctx->bits_per_raw_sample ) > + if ( avctx->bits_per_raw_sample > + && av_pix_fmt_desc_get(avctx->pix_fmt)->comp[0].depth_minus1 > 7) > maxdepth = (1 << avctx->bits_per_raw_sample) - 1;
So bits_per_raw_sample can be != 0 but kind of wrong in some 8 bits cases? Do you have an example for this? + int bps_delta = av_pix_fmt_desc_get(avctx->pix_fmt)->comp[0].depth_minus1 + 1 - avctx->bits_per_raw_sample; + if (!avctx->bits_per_raw_sample || !bps_delta) { Could this be moved out the loop (the compiler should do it, but...) ? Second point, can this get negative? I don't think so because it means something would be seriously wrong. Otherwise, maybe you could do something like: int bps_delta = 0; if ( avctx->bits_per_raw_sample > 0 ) { bps_delta = av_pix_fmt_desc_get(avctx->pix_fmt)->comp[0].depth_minus1 + 1 - avctx->bits_per_raw_sample; if (bps_delta < 0) bps_delta = 0; // maybe unneeded or wrong } [...] if (!bps_delta) + for (j = 0; j < avctx->width; j++) + ((uint16_t *)bytestream)[j] = ((uint16_t *)ptr)[j] >> bps_delta; Is this correct? I mean, is your test case improved? If it is the case, I think "[PATCH 2/5] pnmenc: use bits_per_raw_sample" is wrong then, or there's maybe an endianess issue somewhere. I think during my tests I saw something similar. From: http://netpbm.sourceforge.net/doc/pamendian.html it says p[bgp]m is supposedly bigendian, though at this point anything can happen. Otherwise, this shift would undo scaling performed elsewhere or actually remove bits from the actual value, which would be incorrect. Best regards, -- Christophe _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel