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

Message ID 20210622134652.1279260-1-andrey.konovalov@linaro.org
Headers show
Series
  • qcam: viewfinder_gl: add RAW8, RAW10P and RAW12P formats
Related show

Message

Andrey Konovalov June 22, 2021, 1:46 p.m. UTC 
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.
* 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

Comments

Laurent Pinchart June 30, 2021, 1:17 a.m. UTC | #1 
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'll merge patches 1/9 to 8/9 already, and merge 9/9 or a different fix
once we finish this discussion.

> * 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
> 
>
Andrey Konovalov July 9, 2021, 4:29 p.m. UTC | #2 
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
>>
>>
>