bitbank2 / JPEGDEC

An optimized JPEG decoder for Arduino
Apache License 2.0
366 stars 47 forks source link

Rendering errors of test images #18

Closed bku-sue closed 3 years ago

bku-sue commented 3 years ago

Hi! I'm using this library with an ST7796S LCD display, but there's an issue with the drawing of the images: IMG_20210216_125944 IMG_20210216_130348 IMG_20210216_125443

The tulips are downscaled to half, the others aren't downscaled. This is the integration:


static int JPEGDraw(JPEGDRAW *pDraw)
{
    area_t area;
    area.x1 = pDraw->x;
    area.y1 = pDraw->y;
    area.x2 = pDraw->x + pDraw->iWidth - 1;
    area.y2 = pDraw->y + pDraw->iHeight - 1;
    St7796::instance().SetAddressWindow(area);

    size_t areaSize = (pDraw->iWidth) * (pDraw->iHeight);
    St7796::instance().WriteData(pDraw->pPixels, areaSize * sizeof(*pDraw->pPixels));
    return 1;
}

bool DrawJpeg(const point_t &position, Image *image)
{
    std::unique_ptr<JPEGDEC> jpeg { new JPEGDEC() };
    int options = 0; // or JPEG_SCALE_HALF
    if ((jpeg.get() != nullptr) && (jpeg->openRAM(image->data, image->size, &JPEGDraw)))
    {
        jpeg->setPixelType(RGB565_BIG_ENDIAN);
        bool success = jpeg->decode(position.x, position.y, options) == 1;
        jpeg->close();
        return success;
    }
    else
    {
        return false;
    }
}

Any idea what went wrong here?

bitbank2 commented 3 years ago

I've seen similar issues on these LCDs, you should check:

1) The CS line is being activated/de-activated properly 2) The setWindow() may be 'off-by-1' in it's width/height setting 3) The SPI speed may be too fast.

bku-sue commented 3 years ago

I figured it out, the problem was that there was no synchronization to ensure that the display area was written, before returning from the DRAW callback. Therefore the decoder kept running and overwriting part of the output buffer, which was only afterwards sent to the display with DMA. I've split the transfer in two, now it's working:

static int JPEGDraw(JPEGDRAW *pDraw)
{
    area_t area;
    area.x1 = pDraw->x;
    area.y1 = pDraw->y;
    area.x2 = pDraw->x + pDraw->iWidth - 1;
    area.y2 = pDraw->y + pDraw->iHeight - 1;
    St7796::instance().SetAddressWindow(area);

    assert((pDraw->iHeight % 2) == 0);
    size_t rowsPerTransfer = pDraw->iHeight / 2;
    size_t areaSize = pDraw->iWidth * rowsPerTransfer;
    for (size_t i = 0; i < 2; i++)
    {
        area.y1 = pDraw->y + (rowsPerTransfer * i);
        area.y2 = area.y1 + rowsPerTransfer - 1;

        St7796::instance().DrawArea(area, pDraw->pPixels + (areaSize * i));
    }
    return 1;

I think it would make sense to implement a double buffering scheme inside the library. What do you think?

bitbank2 commented 3 years ago

I think it makes more sense to do double-buffering outside the library since most users won't be affected by this. A simple way to have the DRAW function work on your machine:

1) Wait for previous DMA operation to complete 2) Copy new pixels into private buffer. 3) Send private buffer to screen via DMA