So with this patch, Ori, the 50 images at the link https://thuychi.vn/anh.html now work? Or are your changes improvements to the jpeg decoder that don't handle all of the images on that webpage?
On Sun, Jan 11, 2026, at 12:23 PM, [email protected] wrote: > Quoth [email protected]: >> Quoth [email protected]: >> > Quoth [email protected]: >> > > Quoth [email protected]: >> > > > Hello 9fans, >> > > > For me, jpg(1) can't display most jpeg images I have. So I made a go >> > > > program that convert a jpeg image from stdin to a png image to stdout. >> > > > Assume that Plan 9's png is not broken ;) >> > > > >> > > > To test, go to https://thuychi.vn/anh.html >> > > > Of 50 images, Plan 9's jpg could only decode 3. The go library can >> > > > decode all of them. >> > > > >> > > > I hope that there will be go image/plan9 and a draw library that works >> > > > on Plan 9 too... Replace the jpg(1) programs with the Go version! >> > > > >> > > > bind /bin/jpgtopng.rc /bin/jpg, I think at least abaco don't need to >> > > > be modified. >> > > > >> > > >> > > Great, I think you just reminded me of an old bug in jpg that I'd >> > > been looking into (and then got distracted from). It looks like when >> > > we decode jpg images, we mishandle progresive jpgs that have scans >> > > that don't use interleaving, and have subsampled components. These >> > > aren't too common, but you'll run across them in the wild. >> > > >> > > Normally, we'll end up with the luminance channel coming through, >> > > but the chroma channels completely broken. >> > > Now that I'm looking at it again, I think this has something to do >> > > with how we walk the scans; data either comes in MCU or scan order, >> > > and we may be mangling this. >> > > >> > > this may not be so hard to fix. >> > > >> > >> > Alright, I think I found/fixed up the patch; there were two >> > issues that we had; first, when decoding the DC components, for >> > scans where Ns=1, we need to handle them in raster order, not >> > MCU order[1]. The change to progressivedc() handles the Nf==1 >> > case to deal with the different scan order. >> > >> > The second case was that we don't need to have all frame >> > components present in the scan, and we needed to use Csj (the >> > component selector to pick the appropriate mage components from >> > the scan header. >> > >> > [1] https://www.w3.org/Graphics/JPEG/itu-t81.pdf, A.2.2 >> > [2] https://www.w3.org/Graphics/JPEG/itu-t81.pdf, B.2.3 >> > >> >> Whoops, had an out of bounds write, and now that I'm paging it >> in, I think I want to refactor the change and detangle a bit. > > Ok -- this should be both working, and much easier to understand. > Instead of tangling both Nf=1 and Nf!=1 cases, it splits them; > the two cases naturally stand alone. > > diff 07c4ebae5b344643d4a00b4532d216ef7291e42a uncommitted > --- a/sys/src/cmd/jpg/readjpg.c > +++ b/sys/src/cmd/jpg/readjpg.c > @@ -457,7 +457,7 @@ > return 0; > } > if(Bread(h->fd, tmp, 2) != 2) > - Readerr: > +Readerr: > jpgerror(h, readerr); > n = int2(tmp, 0); > if(n < 2) > @@ -574,7 +574,7 @@ > t->shift[v] = 8-cnt; > t->value[v] = t->val[t->valptr[i]+(code-t->mincode[i])]; > > - continueBytes: > +continueBytes: > v++; > } > > @@ -899,16 +899,15 @@ > void > progressivedc(Header *h, int comp, int Ah, int Al) > { > - int Ns, z, ri, mcu, nmcu; > + int Ns, z, ri, mcu, nmcu; > int block, t, diff, qt, *dc, bn; > + int x, y, H, V, nhor, nver, q; > Huffman *dcht; > uchar *ss; > - int i, Td[3], DC[3], blockno[3]; > + int i, j, Td[4], DC[4], scancomp[4], blockno[4]; > > ss= h->ss; > Ns = ss[0]; > - if(Ns!=1 && Ns!=h->Nf) > - jpgerror(h, "ReadJPG: can't handle progressive with Ns!=1 and > Nf!=Ns > in DC scan"); > > /* initialize data structures */ > h->cnt = 0; > @@ -916,47 +915,81 @@ > h->peek = -1; > > for(i=0; i<Ns; i++){ > - /* > - * JPEG requires scan components to be in same order as in > frame, > - * so if both have 3 we know scan is Y Cb Cr and there's no > need to > - * reorder > - */ > nibbles(ss[2+2*i], &Td[i], &z); /* z is ignored */ > DC[i] = 0; > + for(j=0; j<h->Nf; j++) > + if(h->comp[j].C == ss[1+2*i]){ > + scancomp[i] = j; > + break; > + } > + if(j == h->Nf) > + jpgerror(h, "ReadJPG: bad component selector in scan"); > } > > ri = h->ri; > > - nmcu = h->nacross*h->ndown; > - memset(blockno, 0, sizeof blockno); > - for(mcu=0; mcu<nmcu; ){ > - for(i=0; i<Ns; i++){ > - if(Ns != 1) comp = i; > - > - dcht = &h->dcht[Td[i]]; > - qt = h->qt[h->comp[comp].Tq][0]; > - dc = h->dccoeff[comp]; > - bn = blockno[i]; > - > - for(block=0; block<h->nblock[comp]; block++){ > + if(Ns == 1){ > + /* ITU-T.81 A.2.2: scan in raster order if Ns = 1*/ > + comp = scancomp[0]; > + H = h->comp[comp].H; > + V = h->comp[comp].V; > + q = 8*h->Hmax/H; > + nhor = (h->X+q-1)/q; > + q = 8*h->Vmax/V; > + nver = (h->Y+q-1)/q; > + dcht = &h->dcht[Td[0]]; > + qt = h->qt[h->comp[comp].Tq][0]; > + dc = h->dccoeff[comp]; > + mcu = 0; > + for(y=0; y<nver; y++){ > + for(x=0; x<nhor; x++){ > + bn = (x/H + h->nacross*(y/V))*H*V + H*(y%V) + > x%H; > if(Ah == 0){ > t = decode(h, dcht); > diff = receive(h, t); > - DC[i] += diff; > - dc[bn] = qt*DC[i]<<Al; > + DC[0] += diff; > + dc[bn] = qt*DC[0]<<Al; > }else > dc[bn] |= qt*receivebit(h)<<Al; > - bn++; > + /* process restart marker, if present */ > + mcu++; > + if(ri>0 && mcu%ri==0){ > + restart(h, mcu); > + DC[0] = 0; > + } > } > - blockno[i] = bn; > } > - > - /* process restart marker, if present */ > - mcu++; > - if(ri>0 && mcu<nmcu && mcu%ri==0){ > - restart(h, mcu); > - for(i=0; i<Ns; i++) > - DC[i] = 0; > + }else{ > + /* ITU-T.81 A.2.3: scan in MCU order otherwise */ > + nmcu = h->nacross*h->ndown; > + memset(blockno, 0, sizeof blockno); > + for(mcu=0; mcu<nmcu; ){ > + for(i=0; i<Ns; i++){ > + comp = scancomp[i]; > + dcht = &h->dcht[Td[i]]; > + qt = h->qt[h->comp[comp].Tq][0]; > + dc = h->dccoeff[comp]; > + bn = blockno[i]; > + for(block=0; block<h->nblock[comp]; block++){ > + if(Ah == 0){ > + t = decode(h, dcht); > + diff = receive(h, t); > + DC[i] += diff; > + dc[bn] = qt*DC[i]<<Al; > + }else > + dc[bn] |= qt*receivebit(h)<<Al; > + bn++; > + } > + blockno[i] = bn; > + } > + > + /* process restart marker, if present */ > + mcu++; > + if(ri>0 && mcu<nmcu && mcu%ri==0){ > + restart(h, mcu); > + for(i=0; i<Ns; i++) > + DC[i] = 0; > + } > } > } > } > @@ -1003,7 +1036,7 @@ > mcu = 0; > for(y=0; y<nver; y++){ > for(x=0; x<nhor; x++){ > - /* Figure G-3 */ > + /* Figure G-3 */ > if(eobrun > 0){ > --eobrun; > continue; > @@ -1104,7 +1137,7 @@ > for(x=0; x<nhor; x++){ > /* Figure G-7 */ > > - /* arrange blockno to be in same sequence as original > scan calculation. */ > + /* arrange blockno to be in same sequence as original > scan calculation. */ > tmcu = x/H + (nacross/H)*(y/V); > blockno = tmcu*H*V + H*(y%V) + x%H; > acc = ac[blockno]; > @@ -1237,7 +1270,9 @@ > > static > void > -colormapall1(Header *h, int colorspace, Rawimage *image, int > data0[8*8], int data1[8*8], int data2[8*8], int mcu, int nacross) > +colormapall1(Header *h, int colorspace, Rawimage *image, > + int data0[8*8], int data1[8*8], int data2[8*8], > + int mcu, int nacross) > { > uchar *rpic, *gpic, *bpic, *rp, *gp, *bp; > int *p0, *p1, *p2; > @@ -1290,7 +1325,10 @@ > > static > void > -colormap(Header *h, int colorspace, Rawimage *image, int *data0[8*8], > int *data1[8*8], int *data2[8*8], int mcu, int nacross, int Hmax, int > Vmax, int *H, int *V) > +colormap(Header *h, int colorspace, Rawimage *image, > + int *data0[8*8], int *data1[8*8], int *data2[8*8], > + int mcu, int nacross, int Hmax, int Vmax, > + int *H, int *V) > { > uchar *rpic, *gpic, *bpic; > int x, y, dx, dy, minx, miny; ------------------------------------------ 9fans: 9fans Permalink: https://9fans.topicbox.com/groups/9fans/T33c04c0193c96a35-Ma10da23534ed5ade074e817b Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
