[libcamera-devel] [PATCH v5 0/9] qcam: viewfinder_gl: add RAW8, RAW10P and RAW12P formats

[Andrey Konovalov]

Hi Laurent,

Thanks for merging the first 8 patches from this series!

And sorry for the delayed reply (has just returned from the vacations).

On 30.06.2021 04:17, Laurent Pinchart wrote:
> Hi Andrey,
> 
> Very nice patch series, thank you !
> 
> On Tue, Jun 22, 2021 at 04:46:43PM +0300, Andrey Konovalov wrote:
>> This patchset adds support for both 10-bit, and 12-bit raw Bayer packed
>> and 8-bit raw Bayer formats. GL_RED texture is used to store the image data.
>> Not only this allows to use the same shader code in 10-bit and 12-bit cases.
>> But it also turned out that sampling from GL_RED texture is easier (less
>> "patterns") than from GL_RGB - even though we loose the "auto-skipping the LS
>> bytes of pixel values" feature in the 12-bit case.
>>
>> Changes in v5 vs v4 [1]:
>> * 8-bit raw Bayer format is back, but it is now split into several patches
>>    (4/9 to 8/9) to make it easier to see the particular changes done to
>>    the original shader code.
>>    With the first 8 of the total 9 patches applied I am seeing some
>>    artifacts (note the horizontal colour lines which change when the viewfinder
>>    window height is changed) [2].
>>    Patch 9/9 fixes that, but the root cause of the issue isn't currently known.
>>    That's why 9/9 comes as RFC, and the series can be merged without it.
> 
> I've noticed in
> https://github.com/motmot/libcamiface/blob/master/demo/liveview-glut.c
> that GL_TEXTURE_MIN_FILTER is set to GL_LINEAR, while you set it to
> GL_NEAREST. Could it be related to the artifacts you're seeing ?

I started from GL_TEXTURE_MIN_FILTER set to GL_LINEAR, and saw the similar
artifacts. Then changed it to GL_NEAREST, though it didn't produce a visible
difference (just using a linear mix of pixels of different colours didn't seem
like a good idea for me).
I've got one more thing to check, and will let you know if it brings any new
information after I am done with it.

> I'll merge patches 1/9 to 8/9 already, and merge 9/9 or a different fix
> once we finish this discussion.

Sounds good!

Thanks,
Andrey

>> * s/tex_raw/tex_y in the shaders to match the uniform name used in the cpp
>>    code.
>>
>> [1] https://patchwork.libcamera.org/cover/12629/
>> [2] https://people.linaro.org/~andrey.konovalov/cam-56/20210622.imx219-RAW8-artifacts.viewfinder_gl-add-RAW1xP-and-RAW8-support-v4.1-raw8.0.mp4
>>
>>
>