minor bug report: png spews

minor bug report: png spews

[Ethan Gardener]

png makes a nuisance of itself by writing confusing near-useless non-error data to stderr in a very un-unixlike manner. it can be a nuisance in a pipeline. jpg behaves correctly. 

term% png -9 img/timmy-arch-2017-03-12.png		| awk '/^compressed$/ {next}; {print $4-$2, $5-$3; exit}'
png: writing 3888000 bytes for len 1296000 chan m8
1440 900
term% jpg -9 img/Carina_Peach_4553.jpg			| awk '/^compressed$/ {next}; {print $4-$2, $5-$3; exit}'
4553 5004

Re: [9front] minor bug report: png spews

[Ethan Gardener]

looks like it's just a forgotten debug print because it only occurs with -9. oddly, there are rather a lot of lines computing just for that message. 

here's a patch, tested minimally. let me know if you want a different format; this is diff -c.

png.c.0:132,138 - png.c:132,137
  	Rawimage **array, *r, *c;
  	Image *i, *i2;
  	int j, ch, outchan;
- 	long len;
  	Biobuf b;
  	char buf[32];
  	static int inited;
png.c.0:207,230 - png.c:206,211
  	}
  	if(nineflag){
  		chantostr(buf, outchan);
- 		len = (c->r.max.x - c->r.min.x) * (c->r.max.y - c->r.min.y);
- 		switch(c->chandesc){
- 		case CY:
- 			// len *= 1;
- 			break;
- 		case CYA16:
- 			len *= 2;
- 			break;
- 		case CRGB24:
- 			len *= 3;
- 			break;
- 		case CRGBA32:
- 			len *= 4;
- 			break;
- 		}
- 		if(c->chanlen != len)
- 			fprint(2, "%s: writing %d bytes for len %ld chan %s\n",
- 				argv0, c->chanlen, len, buf);
  		print("%11s %11d %11d %11d %11d ", buf,
  			c->r.min.x, c->r.min.y, c->r.max.x, c->r.max.y);
  		if(write(1, c->chans[0], c->chanlen) != c->chanlen){

Re: [9front] minor bug report: png spews

[ori]

> looks like it's just a forgotten debug print because it only occurs with -9. oddly, there are rather a lot of lines computing just for that message. 
> 
> here's a patch, tested minimally. let me know if you want a different format; this is diff -c.
> 
> png.c.0:132,138 - png.c:132,137
>   	Rawimage **array, *r, *c;
>   	Image *i, *i2;
>   	int j, ch, outchan;
> - 	long len;
>   	Biobuf b;
>   	char buf[32];
>   	static int inited;
> png.c.0:207,230 - png.c:206,211
>   	}
>   	if(nineflag){
>   		chantostr(buf, outchan);
> - 		len = (c->r.max.x - c->r.min.x) * (c->r.max.y - c->r.min.y);
> - 		switch(c->chandesc){
> - 		case CY:
> - 			// len *= 1;
> - 			break;
> - 		case CYA16:
> - 			len *= 2;
> - 			break;
> - 		case CRGB24:
> - 			len *= 3;
> - 			break;
> - 		case CRGBA32:
> - 			len *= 4;
> - 			break;
> - 		}
> - 		if(c->chanlen != len)
> - 			fprint(2, "%s: writing %d bytes for len %ld chan %s\n",
> - 				argv0, c->chanlen, len, buf);
>   		print("%11s %11d %11d %11d %11d ", buf,
>   			c->r.min.x, c->r.min.y, c->r.max.x, c->r.max.y);
>   		if(write(1, c->chans[0], c->chanlen) != c->chanlen){

Hm. Does this happen on all PNG images?
If not, do you have an example image that triggers it?

I haven't tested yet, but it looks like it's warning about
a possible miscomputation of image size.

Re: [9front] minor bug report: png spews

[Ethan Gardener]

On Fri, Aug 21, 2020, at 7:47 PM, ori@eigenstate.org wrote:
> 
> Hm. Does this happen on all PNG images?
> If not, do you have an example image that triggers it?

good question. it happens on all 3 png images i have on my 9front box. not a large sample, i know. at least 2 of them were produced by topng.

here's 2 on the web:

hget http://spazient.thecomicseries.com/images/comics/190/44807a1588321366b69316f455407560.png | png -9 > /dev/null
hget http://spazient.thecomicseries.com/images/comics/190/44807a1587857865b69316f137336022.png | png -9 > /dev/null

so it's still every image i've tried. 

> I haven't tested yet, but it looks like it's warning about
> a possible miscomputation of image size.

then why can't it happen for any case other than nineflag (-9), and why can't it happen with jpg's nineflag? i honestly think the calculation of len is just wrong, and if it were correct we'd never see the message.

Re: [9front] minor bug report: png spews

[Ethan Gardener]

On Fri, Aug 21, 2020, at 8:25 PM, Ethan Gardener wrote:
> i honestly think the calculation 
> of len is just wrong, 

or that comparing len with c->chanlen is just spurious.

Re: [9front] minor bug report: png spews

[Alex Musolino]

The value of c->chandesc for the images you linked to is actually CRGBV
which doesn't appear in the switch statement.  So, I'd say your patch
is fine.