[FFmpeg-devel] [PATCH]Fix pnm encoding with bpc set

Christophe Gisquet christophe.gisquet at gmail.com
Sun Aug 24 12:28:43 CEST 2014


Hi,

2014-08-24 12:13 GMT+02:00 Carl Eugen Hoyos <cehoyos at 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


More information about the ffmpeg-devel mailing list