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 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,18 +899,17 @@ void progressivedc(Header *h, int comp, int Ah, int Al) { - int Ns, z, ri, mcu, nmcu; - int block, t, diff, qt, *dc, bn; + int Ns, z, ri, mcu, nmcu, bn, block; + int t, qt, *dc, x, y, H, V, nhor, 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"); + H = V = nhor = 0; - /* initialize data structures */ + ss = h->ss; + Ns = ss[0]; + ri = h->ri; h->cnt = 0; h->sr = 0; h->peek = -1; @@ -917,43 +916,66 @@ 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 + * scan component specifier Csj identifies component by selector, + * not position; a scan might not have all frame components. */ - nibbles(ss[2+2*i], &Td[i], &z); /* z is ignored */ + nibbles(ss[2+2*i], &Td[i], &z); DC[i] = 0; + blockno[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; - + /* A.2.2: scan in raster order for Ns = 1 */ 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; + 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; + nmcu = h->nacross*H*h->ndown*V; + } - dcht = &h->dcht[Td[i]]; - qt = h->qt[h->comp[comp].Tq][0]; - dc = h->dccoeff[comp]; - bn = blockno[i]; + dcht = &h->dcht[Td[0]]; + qt = h->qt[h->comp[scancomp[0]].Tq][0]; + dc = h->dccoeff[scancomp[0]]; - 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++; + for(mcu=0; mcu<nmcu; ){ + if(Ns == 1){ + x = mcu % nhor; + y = mcu / nhor; + bn = (x/H + h->nacross*(y/V))*H*V + H*(y%V) + x%H; + if(Ah == 0){ + t = decode(h, dcht); + DC[0] += receive(h, t); + dc[bn] = qt*DC[0]<<Al; + }else + dc[bn] |= qt*receivebit(h)<<Al; + }else{ + 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]; + for(block=0; block<h->nblock[comp]; block++){ + bn = blockno[i]++; + if(Ah == 0){ + t = decode(h, dcht); + DC[i] += receive(h, t); + dc[bn] = qt*DC[i]<<Al; + }else + dc[bn] |= qt*receivebit(h)<<Al; + } } - blockno[i] = bn; } - - /* process restart marker, if present */ mcu++; - if(ri>0 && mcu<nmcu && mcu%ri==0){ + if(ri > 0 && mcu < nmcu && mcu%ri == 0){ restart(h, mcu); for(i=0; i<Ns; i++) DC[i] = 0; @@ -1003,7 +1025,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 +1126,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 +1259,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 +1314,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-Ma142ea0e03007ef0a83d1315 Delivery options: https://9fans.topicbox.com/groups/9fans/subscription
