Skip to content
This repository has been archived by the owner on Feb 16, 2023. It is now read-only.

vid: remove pixel formats RGB555 and RGB565 #29

Merged
merged 1 commit into from
Mar 6, 2022
Merged

vid: remove pixel formats RGB555 and RGB565 #29

merged 1 commit into from
Mar 6, 2022

Conversation

alfredh
Copy link
Contributor

@alfredh alfredh commented Mar 4, 2022

proposal to remove pixel formats with low bit depth

@sreimers sreimers merged commit f7c2227 into main Mar 6, 2022
@sreimers sreimers deleted the vid_rgb565 branch March 6, 2022 06:51
@juha-h
Copy link
Contributor

juha-h commented Mar 12, 2022

I was using VID_FMT_RGB565 in my Android app. With what format should I replace it?

@juha-h
Copy link
Contributor

juha-h commented Mar 12, 2022

I tried with VID_FMT_RGB32 and VID_FMT_YUV444P. With VID_FMT_RGB32 there was colorful garbage on the screen and with VID_FMT_YUV444P screen stayed black. Please bring the removed format back unless a working alternative is found.

The code where I use the format goes like this:

int opengles_display(struct vidisp_st *st, const char *title, const struct vidframe *frame, uint64_t timestamp)
{
    (void)title;
    (void)timestamp;
    int err;

    /* This is hack to make sure that context is initialised on the same thread */
    if (!st->context) {
        err = context_initialize(st);
        if (err) {
            LOGW("Renderer context init failed with error %d\n", err);
            return err;
        }
    }

    if (!st->vf) {
        if (frame->size.w & 3) {
            LOGW("opengles_display: frame width must be multiple of 4\n");
            return EINVAL;
        }

        err = vidframe_alloc(&st->vf, VID_FMT_RGB565, &frame->size);
        if (err) {
            LOGW("opengles_display: vidframe_alloc failed\n");
            return err;
        }
    }

    vidconv(st->vf, frame, NULL);

    opengles_render(st);

    err = eglSwapBuffers(st->display, st->surface);
    if (!err)
        LOGW("eglSwapBuffers() returned error %s\n", egl_error(eglGetError()));

    return err;
}

@juha-h
Copy link
Contributor

juha-h commented Mar 12, 2022

Tried all formats bpp >= 16 and all of them failed to work. Either colorful garbage or black screen.

@juha-h
Copy link
Contributor

juha-h commented Mar 12, 2022

With VID_FMT_YUV444P I get errors:

03-12 11:38:58.176  3737  3805 D Baresip+ Lib: vidconv: no pixel converter found for yuv420p -> yuv444p

And with VID_FMT_YUV420P I see the screen split into 4 colorful garbage rectangles.

@sreimers
Copy link
Member

Looks like these are the only supported formats on android:

https://developer.android.com/reference/android/graphics/Bitmap.Config

If I interpret the documention correctly the prefered one is:

https://developer.android.com/reference/android/graphics/Bitmap.Config#ARGB_8888

https://developer.android.com/reference/android/graphics/Bitmap.Config#RGB_565
Says This configuration can produce slight visual artifacts depending on the configuration of the source

@sreimers
Copy link
Member

Can you try VID_FMT_ARGB?

@juha-h
Copy link
Contributor

juha-h commented Mar 12, 2022

With VID_FMT_ARGB I get black screen and lots of these to log:

03-12 13:22:02.818  6879  6975 D Baresip+ Lib: vidconv: no pixel converter found for yuv420p -> argb
03-12 13:22:02.854  6879  6975 D Baresip+ Lib: vidconv: no pixel converter found for yuv420p -> argb
03-12 13:22:02.869  6879  6975 D Baresip+ Lib: vidconv: no pixel converter found for yuv420p -> argb

Am I missing some module that does the conversion?

@sreimers
Copy link
Member

I see, we have no conversion function:

https://github.com/baresip/rem/blob/main/src/vidconv/vconv.c#L644-L661

@juha-h
Copy link
Contributor

juha-h commented Mar 12, 2022

So best would be to add support for ARGB_8888 or (if it takes long or cannot be done at all) restore support for VID_FMT_RGB565.

@juha-h
Copy link
Contributor

juha-h commented Mar 12, 2022

There has been reports on the net that ARGB_8888 performance can be very bad. So better to stick to VID_FMT_RGB565.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants