Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[rfc/wip] use v4l2 format info instead #420

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from
Draft

[rfc/wip] use v4l2 format info instead #420

wants to merge 5 commits into from

Conversation

vicamo
Copy link
Contributor

@vicamo vicamo commented Mar 24, 2021 •
edited
Loading

This is an attempt to remove struct v4l2l_format and use in kernel struct v4l2_format_info.

While moving toward videobuf2 framework, I'd like to reuse more and more already in kernel APIs as possible. This PR is going to change:

  • how formats array is created:
    formats array used to carry fourcc, bpp info, etc., but it may not be synced with in kernel definitions and it's missing support for multiplane as well. While using videobuf2, we'd like to make sure there is no unexpected mismatch, especially in calculating buffer size. The formats array is now the one originally defined in v4l2_format_info() moved to the global namespace with additional entries added.

  • format descriptions:
    kernel can help filling format description/flags since v4.2. Use the in kernel helper so that we can have identical names as other V4L2 based drivers do.

  • supported formats set:

    note that following formats from the original set have no corresponding v4l2_format_info entry:

    V4L2_PIX_FMT_DV
V4L2_PIX_FMT_H263
V4L2_PIX_FMT_H264
V4L2_PIX_FMT_H264_MVC
V4L2_PIX_FMT_H264_NO_SC
V4L2_PIX_FMT_HEVC
V4L2_PIX_FMT_JPEG
V4L2_PIX_FMT_MJPEG
V4L2_PIX_FMT_MPEG
V4L2_PIX_FMT_MPEG1
V4L2_PIX_FMT_MPEG2
V4L2_PIX_FMT_MPEG4
V4L2_PIX_FMT_RGB332
V4L2_PIX_FMT_RGB555X
V4L2_PIX_FMT_RGB565X
V4L2_PIX_FMT_VC1_ANNEX_G
V4L2_PIX_FMT_VC1_ANNEX_L
V4L2_PIX_FMT_VP8
V4L2_PIX_FMT_VP9
V4L2_PIX_FMT_XVID
V4L2_PIX_FMT_Y10
V4L2_PIX_FMT_Y12
V4L2_PIX_FMT_Y16
V4L2_PIX_FMT_Y4
V4L2_PIX_FMT_Y41P
V4L2_PIX_FMT_Y6
V4L2_PIX_FMT_YUV32
V4L2_PIX_FMT_YUV444
V4L2_PIX_FMT_YUV555
V4L2_PIX_FMT_YUV565
V4L2_PIX_FMT_YYUV

Note this PR is not yet complete due the the lack of many format info entries in the implementation of v4l2_format_info even in current v5.12 kernel source. To be done some time later.

@vicamo vicamo marked this pull request as draft March 24, 2021 15:07
@vicamo vicamo force-pushed the for-upstream/use-v4l2_format_info-instead branch 3 times, most recently from 484ac49 to 1b95bba Compare March 24, 2021 15:26
@vicamo
Copy link
Contributor Author

vicamo commented Mar 24, 2021 

=== Testing 4.4.0-148-generic ===
Building v4l2-loopback driver...
make -C /lib/modules/4.4.0-148-generic/build M=/__w/v4l2loopback/v4l2loopback modules
make[1]: Entering directory `/usr/src/linux-headers-4.4.0-148-generic'
  CC [M]  /__w/v4l2loopback/v4l2loopback/v4l2loopback.o
  Building modules, stage 2.
  MODPOST 1 modules
  CC      /__w/v4l2loopback/v4l2loopback/v4l2loopback.mod.o
  LD [M]  /__w/v4l2loopback/v4l2loopback/v4l2loopback.ko
make[1]: Leaving directory `/usr/src/linux-headers-4.4.0-148-generic'
make -C utils
make[1]: Entering directory `/__w/v4l2loopback/v4l2loopback/utils'
cc  -I..   v4l2loopback-ctl.c   -o v4l2loopback-ctl
In file included from v4l2loopback-ctl.c:25:0:
../v4l2loopback_formats.h:131:2: error: initializer element is not constant
  V4L2_PIX_FMT_ARGB555X,
  ^
../v4l2loopback_formats.h:131:2: error: (near initialization for 'formats[23]')
../v4l2loopback_formats.h:135:2: error: initializer element is not constant
  V4L2_PIX_FMT_XRGB555X,
  ^
../v4l2loopback_formats.h:135:2: error: (near initialization for 'formats[24]')
../v4l2loopback_formats.h:223:2: error: initializer element is not constant
  V4L2_PIX_FMT_Y16_BE,
  ^
../v4l2loopback_formats.h:223:2: error: (near initialization for 'formats[46]')
../v4l2loopback_formats.h: In function 'backport_v4l_fill_fmtdesc':
../v4l2loopback_formats.h:1108:2: error: case label does not reduce to an integer constant
  case V4L2_PIX_FMT_ARGB555X: descr = "16-bit ARGB 1-5-5-5 BE"; break;
  ^
../v4l2loopback_formats.h:1109:2: error: case label does not reduce to an integer constant
  case V4L2_PIX_FMT_XRGB555X: descr = "16-bit XRGB 1-5-5-5 BE"; break;
  ^
../v4l2loopback_formats.h:1131:2: error: case label does not reduce to an integer constant
  case V4L2_PIX_FMT_Y16_BE: descr = "16-bit Greyscale BE"; break;
  ^
v4l2loopback-ctl.c: In function 'print_conf':
v4l2loopback-ctl.c:436:3: warning: format '%s' expects argument of type 'char *', but argument 2 has type 'struct v4l2_loopback_config *' [-Wformat=]
   printf("configuration: %s\n", cfg);
   ^
v4l2loopback-ctl.c: In function 'parse_caps':
v4l2loopback-ctl.c:656:6: warning: format '%c' expects argument of type 'char *', but argument 3 has type 'char (*)[5]' [-Wformat=]
      &caps->height, &caps->fps_num, &caps->fps_denom) <= 0) {
      ^
make[1]: *** [v4l2loopback-ctl] Error 1
make[1]: Leaving directory `/__w/v4l2loopback/v4l2loopback/utils'
make: *** [utils/v4l2loopback-ctl] Error 2

Interesting.

@umlaeute umlaeute force-pushed the for-upstream/use-v4l2_format_info-instead branch from 8b5eb97 to 09f82ec Compare March 24, 2021 16:57
@umlaeute
Copy link
Owner

is this marked as draft because of the missing formats, or because it failed to build in the CI?

@vicamo
Copy link
Contributor Author

vicamo commented Mar 24, 2021

is this marked as draft because of the missing formats, or because it failed to build in the CI?

Missing format info.

@vicamo vicamo force-pushed the for-upstream/use-v4l2_format_info-instead branch from 09f82ec to 7c57445 Compare March 24, 2021 17:44
@vicamo
Copy link
Contributor Author

vicamo commented Mar 24, 2021 •
edited
Loading

@umlaeute it seems I overwritten your commit :)

Still a draft. Need to address those missing format info entries.

@vicamo
Copy link
Contributor Author

vicamo commented Mar 25, 2021

Still a draft. Need to address those missing format info entries.

There are 147 formats not currently supported by v4l2_format_info(). That's probably too many to finish them in a short time. Taking V4L2_PIX_FMT_MPEG for example, it may even take a completely different way to calculate framesize than that provided by v4l2_fill_pixfmt().

We can shrink supported formats array to contain only supported ones.

@vicamo vicamo force-pushed the for-upstream/use-v4l2_format_info-instead branch from 7c57445 to 7cfb7b0 Compare March 26, 2021 07:03
@vicamo
Copy link
Contributor Author

vicamo commented Mar 26, 2021

update: sort probed file paths to ensure a stable list is generated.

@vicamo vicamo force-pushed the for-upstream/use-v4l2_format_info-instead branch 8 times, most recently from 4ea388f to 48beb94 Compare March 28, 2021 17:23
@vicamo
vidioc: skip format name/flags filling on kernel >= 4.2
09e112a 
Since all known formats are defined, also removed ifdef-endif
preprocessor directives from formats array.

Signed-off-by: You-Sheng Yang <vicamo@gmail.com>
@vicamo vicamo force-pushed the for-upstream/use-v4l2_format_info-instead branch from 71c4331 to 09e112a Compare March 29, 2021 03:59
@vicamo
utils: use v4l_fill_fmtdesc to retrieve format name
608f867 
Signed-off-by: You-Sheng Yang <vicamo@gmail.com>
@vicamo
compliance: use kernel v4l2_fill_pixfmt()
c10b471 
Signed-off-by: You-Sheng Yang <vicamo@gmail.com>
@vicamo
compliance: drop v4l2l_format in favor of kernel v4l2_format_info
18aa16b 
In this change we expost formats array originally embedded in
v4l2_format_info() as the formats we support. It follows:

  1. v4l2l_format can now be purged,

  2. while formats is also referenced in v4l2loopback-ctl, it has to
     adopt a few changes to enable compiling out of kernel,

  3. v4l2_format_info() and v4l2_fill_pixfmt() shall be overridden
     always as it's to reference our own customized formats array.

  4. v4l2_format_block_width()/v4l2_format_block_height() was defined
     in v4l2-commons.c statically, so there is no symbol collision
     problem, therefore no need to override.

Signed-off-by: You-Sheng Yang <vicamo@gmail.com>
@vicamo
wip: formats: add more
586f447 
Signed-off-by: You-Sheng Yang <vicamo@gmail.com>
@vicamo
Copy link
Contributor Author

vicamo commented Mar 29, 2021

Update:

  • expose formats array originally embedded in v4l2_format_info(), so that we have a sync-ed formats array shared globally.
  • always override v4l2_format_info() and v4l2_fill_pixfmt() for we're to reference the global supported formats array
  • use #ifndef ... instead of #if !defined(...) in defining V4L2_FOO_FMT_BAR.
  • add a few more format info entries to formats array

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vicamo @umlaeute