Skip to content

Commit 7247c26

Browse files
committed
treewide: Use pre-commit utility and replace checkstyle.py
Replace the custom checkstyle.py script with a pre-commit configuration that runs clang-format for C/C++, ruff check and ruff format for Python, shellcheck for shell scripts, and gitlint for commit messages. Update the GitHub Actions style checker workflow to use the pre-commit action instead of running checkstyle.py and ruff separately. Apply formatting fixes across the codebase to conform to the updated clang-format and ruff rules. Add a pre-commit-hook wrapper script in utils/ that runs pre-commit with --show-diff-on-failure. Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
1 parent 9940d72 commit 7247c26

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+616
-1230
lines changed

.clang-format

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ BraceWrapping:
3535
SplitEmptyFunction: true
3636
SplitEmptyRecord: true
3737
SplitEmptyNamespace: true
38-
AfterNamespace: false
38+
AfterNamespace: true
3939
AfterClass: true
4040
BreakBeforeBinaryOperators: None
4141
BreakBeforeInheritanceComma: false
@@ -46,7 +46,7 @@ BreakAfterJavaFieldAnnotations: false
4646
BreakStringLiterals: false
4747
CommentPragmas: '^ IWYU pragma:'
4848
CompactNamespaces: false
49-
PackConstructorInitializers: NextLineOnly
49+
PackConstructorInitializers: BinPack
5050
Cpp11BracedListStyle: false
5151
DerivePointerAlignment: false
5252
DisableFormat: false

.github/workflows/libpisp-style-checker.yml

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,17 @@
44
name: libpisp style checker
55
on:
66
pull_request:
7-
branches: [ main ]
7+
branches: [main]
88

99
jobs:
1010
style-check:
11-
12-
runs-on: [ ubuntu-latest ]
13-
11+
runs-on: ubuntu-latest
1412
steps:
15-
- uses: actions/checkout@v3
16-
with:
17-
fetch-depth: 0
18-
clean: true
19-
20-
- name: Install ruff
21-
run: pip install ruff
22-
23-
- name: Ruff check
24-
run: ruff check utils/
25-
26-
- name: Ruff format
27-
run: ruff format --check utils/
28-
29-
- name: Check style
30-
run: ${{github.workspace}}/utils/checkstyle.py $(git log --format=%P -1 | awk '{print $1 ".." $2}')
13+
- uses: actions/checkout@v4
14+
- uses: actions/setup-python@v5
15+
with:
16+
python-version: '3.x'
17+
- name: Run pre-commit
18+
uses: pre-commit/action@v3.0.1
19+
with:
20+
extra_args: --all-files --show-diff-on-failure

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
build
1+
build

.gitlint

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[general]
2+
contrib=contrib-body-requires-signed-off-by
3+
4+
[title-max-length]
5+
line-length=80
6+
7+
[body-max-line-length]
8+
line-length=80

.pre-commit-config.yaml

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
default_stages: [pre-commit]
2+
3+
repos:
4+
- repo: https://github.com/pre-commit/pre-commit-hooks
5+
rev: v6.0.0
6+
hooks:
7+
- id: trailing-whitespace
8+
- id: end-of-file-fixer
9+
- id: check-json
10+
11+
- repo: https://github.com/pre-commit/mirrors-clang-format
12+
rev: v22.1.3
13+
hooks:
14+
- id: clang-format
15+
types_or: [c, c++]
16+
exclude: '(pisp_be_config\.h|pisp_common\.h|pisp_fe_config\.h|pisp_statistics\.h)$'
17+
18+
- repo: https://github.com/astral-sh/ruff-pre-commit
19+
rev: v0.15.9
20+
hooks:
21+
- id: ruff-check
22+
args: [--fix]
23+
- id: ruff-format
24+
25+
- repo: https://github.com/shellcheck-py/shellcheck-py
26+
rev: v0.11.0.1
27+
hooks:
28+
- id: shellcheck
29+
30+
- repo: local
31+
hooks:
32+
- id: git-diff
33+
name: git diff
34+
entry: git diff --exit-code
35+
language: system
36+
pass_filenames: false
37+
always_run: true
38+
39+
- repo: https://github.com/jorisroovers/gitlint
40+
rev: v0.19.1
41+
hooks:
42+
- id: gitlint
43+
stages: [commit-msg]

LICENSES/GPL-2.0-or-later.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ GNU GENERAL PUBLIC LICENSE
22

33
Version 2, June 1991
44

5-
Copyright (C) 1989, 1991 Free Software Foundation, Inc.
5+
Copyright (C) 1989, 1991 Free Software Foundation, Inc.
66

77
51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
88

LICENSES/Linux-syscall-note.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ NOTE! This copyright does *not* cover user programs that use kernel services by
22

33
Also note that the only valid version of the GPL as far as the kernel is concerned is _this_ particular version of the license (ie v2, not v2.2 or v3.x or whatever), unless explicitly otherwise stated.
44

5-
Linus Torvalds
5+
Linus Torvalds

README.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,5 +69,15 @@ To test the plugin without installing:
6969
GST_PLUGIN_PATH=<build_dir>/src/gst gst-inspect-1.0 pispconvert
7070
```
7171

72+
## For Developers
73+
74+
This project uses [pre-commit](https://pre-commit.com/) to run formatting and linting checks on each commit. To install:
75+
76+
```sh
77+
pip install pre-commit
78+
pre-commit install
79+
pre-commit install --hook-type commit-msg
80+
```
81+
7282
## License
7383
Copyright © 2023, Raspberry Pi Ltd. Released under the BSD-2-Clause License.

src/examples/convert.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ void write_yuv422i(std::ofstream &out, std::array<uint8_t *, 3> &mem, unsigned i
155155
write_plane(out, mem[0], width * 2, height, file_stride, buffer_stride);
156156
}
157157

158+
// clang-format off
158159
struct FormatFuncs
159160
{
160161
std::function<void(const std::array<uint8_t *, 3> &, std::ifstream &, unsigned int, unsigned int, unsigned int,
@@ -173,6 +174,7 @@ const std::map<std::string, FormatFuncs> Formats =
173174
{ "YUYV", { read_yuv422i, write_yuv422i } },
174175
{ "UYVY", { read_yuv422i, write_yuv422i } },
175176
};
177+
// clang-format on
176178

177179
struct Format
178180
{
@@ -218,6 +220,7 @@ int main(int argc, char *argv[])
218220

219221
cxxopts::Options options(argv[0], "PiSP Image Converter");
220222

223+
// clang-format off
221224
options.add_options()
222225
("input", "Input file", cxxopts::value<std::string>())
223226
("output", "Output file", cxxopts::value<std::string>())
@@ -229,6 +232,7 @@ int main(int argc, char *argv[])
229232
("l,list", "Enumerate the media device nodes")
230233
("h,help", "Print usage")
231234
;
235+
// clang-format on
232236

233237
options.parse_positional({ "input", "output" });
234238
options.positional_help("<input file> <output file>");
@@ -376,8 +380,8 @@ int main(int argc, char *argv[])
376380
exit(-1);
377381
}
378382

379-
std::cerr << "Reading " << input_filename << " "
380-
<< in_file.width << ":" << in_file.height << ":" << in_file.stride << ":" << in_file.format << std::endl;
383+
std::cerr << "Reading " << input_filename << " " << in_file.width << ":" << in_file.height << ":" << in_file.stride
384+
<< ":" << in_file.format << std::endl;
381385

382386
{
383387
Buffer::Sync input(buffers.at("pispbe-input"), Buffer::Sync::Access::ReadWrite);

src/gst/gstpispconvert.cpp

Lines changed: 37 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@ GST_DEBUG_CATEGORY_STATIC(gst_pisp_convert_debug);
3434
/* Supported GStreamer formats */
3535
#define PISP_FORMATS "{ RGB, RGBx, BGRx, I420, YV12, Y42B, Y444, YUY2, UYVY, NV12, NV12_128C8, NV12_10LE32_128C8 }"
3636
/* Supported DRM fourccs */
37-
#define PISP_DRM_FORMATS "{ RG24, XB24, XR24, YU12, YV12, YU16, YU24, YUYV, UYVY, NV12, NV12:0x0700000000000004, P030:0x0700000000000004 }"
37+
#define PISP_DRM_FORMATS \
38+
"{ RG24, XB24, XR24, YU12, YV12, YU16, YU24, YUYV, UYVY, NV12, NV12:0x0700000000000004, P030:0x0700000000000004 }"
3839

39-
#define PISP_SRC_CAPS \
40-
"video/x-raw(memory:DMABuf), format=(string)DMA_DRM, drm-format=(string)" PISP_DRM_FORMATS \
41-
", width=(int)[1,32768], height=(int)[1,32768], framerate=(fraction)[0/1,2147483647/1]" \
42-
";" \
43-
GST_VIDEO_CAPS_MAKE_WITH_FEATURES(GST_CAPS_FEATURE_MEMORY_DMABUF, PISP_FORMATS) ";" \
44-
GST_VIDEO_CAPS_MAKE(PISP_FORMATS)
40+
#define PISP_SRC_CAPS \
41+
"video/x-raw(memory:DMABuf), format=(string)DMA_DRM, drm-format=(string)" PISP_DRM_FORMATS \
42+
", width=(int)[1,32768], height=(int)[1,32768], framerate=(fraction)[0/1,2147483647/1]" \
43+
";" GST_VIDEO_CAPS_MAKE_WITH_FEATURES(GST_CAPS_FEATURE_MEMORY_DMABUF, \
44+
PISP_FORMATS) ";" GST_VIDEO_CAPS_MAKE(PISP_FORMATS)
4545

4646
static GstStaticPadTemplate sink_template = GST_STATIC_PAD_TEMPLATE(
4747
"sink", GST_PAD_SINK, GST_PAD_ALWAYS,
@@ -53,13 +53,11 @@ static GstStaticPadTemplate sink_template = GST_STATIC_PAD_TEMPLATE(
5353
/* System memory */
5454
GST_VIDEO_CAPS_MAKE(PISP_FORMATS)));
5555

56-
static GstStaticPadTemplate src0_template = GST_STATIC_PAD_TEMPLATE(
57-
"src0", GST_PAD_SRC, GST_PAD_ALWAYS,
58-
GST_STATIC_CAPS(PISP_SRC_CAPS));
56+
static GstStaticPadTemplate src0_template =
57+
GST_STATIC_PAD_TEMPLATE("src0", GST_PAD_SRC, GST_PAD_ALWAYS, GST_STATIC_CAPS(PISP_SRC_CAPS));
5958

60-
static GstStaticPadTemplate src1_template = GST_STATIC_PAD_TEMPLATE(
61-
"src1", GST_PAD_SRC, GST_PAD_ALWAYS,
62-
GST_STATIC_CAPS(PISP_SRC_CAPS));
59+
static GstStaticPadTemplate src1_template =
60+
GST_STATIC_PAD_TEMPLATE("src1", GST_PAD_SRC, GST_PAD_ALWAYS, GST_STATIC_CAPS(PISP_SRC_CAPS));
6361

6462
#define gst_pisp_convert_parent_class parent_class
6563
G_DEFINE_TYPE(GstPispConvert, gst_pisp_convert, GST_TYPE_ELEMENT);
@@ -154,9 +152,9 @@ static const char *colorimetry_to_pisp(const GstVideoColorimetry *colorimetry)
154152
}
155153

156154
/* Configure colour space conversion blocks for the backend */
157-
static uint32_t configure_colour_conversion(libpisp::BackEnd *backend, const char *in_format,
158-
const char *in_colorspace, const char *out_format,
159-
const char *out_colorspace, unsigned int output_index)
155+
static uint32_t configure_colour_conversion(libpisp::BackEnd *backend, const char *in_format, const char *in_colorspace,
156+
const char *out_format, const char *out_colorspace,
157+
unsigned int output_index)
160158
{
161159
uint32_t rgb_enables = 0;
162160

@@ -177,8 +175,7 @@ static uint32_t configure_colour_conversion(libpisp::BackEnd *backend, const cha
177175
backend->SetCsc(output_index, csc);
178176
rgb_enables |= PISP_BE_RGB_ENABLE_CSC(output_index);
179177
}
180-
else if (g_str_equal(out_format, "RGB888") ||
181-
g_str_equal(out_format, "RGBX8888") ||
178+
else if (g_str_equal(out_format, "RGB888") || g_str_equal(out_format, "RGBX8888") ||
182179
g_str_equal(out_format, "XRGB8888"))
183180
{
184181
/* R/B channel swap to match GStreamer/DRM byte ordering */
@@ -252,21 +249,18 @@ static void gst_pisp_convert_class_init(GstPispConvertClass *klass)
252249
gobject_class, PROP_CROP,
253250
g_param_spec_string("crop", "Crop region for all outputs",
254251
"Crop region as 'x,y,width,height' applied to all outputs (0,0,0,0 = full input)",
255-
"0,0,0,0",
256-
static_cast<GParamFlags>(G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)));
252+
"0,0,0,0", static_cast<GParamFlags>(G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)));
257253

258254
g_object_class_install_property(
259255
gobject_class, PROP_CROP0,
260256
g_param_spec_string("crop0", "Crop region for output 0",
261-
"Crop region as 'x,y,width,height' (0,0,0,0 = no crop / full input)",
262-
"0,0,0,0",
257+
"Crop region as 'x,y,width,height' (0,0,0,0 = no crop / full input)", "0,0,0,0",
263258
static_cast<GParamFlags>(G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)));
264259

265260
g_object_class_install_property(
266261
gobject_class, PROP_CROP1,
267262
g_param_spec_string("crop1", "Crop region for output 1",
268-
"Crop region as 'x,y,width,height' (0,0,0,0 = no crop / full input)",
269-
"0,0,0,0",
263+
"Crop region as 'x,y,width,height' (0,0,0,0 = no crop / full input)", "0,0,0,0",
270264
static_cast<GParamFlags>(G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)));
271265

272266
element_class->change_state = GST_DEBUG_FUNCPTR(gst_pisp_convert_change_state);
@@ -533,8 +527,8 @@ static gboolean parse_input_caps(GstPispConvert *self, GstCaps *caps)
533527
self->priv->in_colorspace = colorimetry_to_pisp(&colorimetry);
534528

535529
GST_INFO_OBJECT(self, "Input DMA-DRM format: drm-format=%s, pisp=%s, colorspace=%s (matrix=%d, range=%d)",
536-
drm_format, self->priv->in_format, self->priv->in_colorspace,
537-
colorimetry.matrix, colorimetry.range);
530+
drm_format, self->priv->in_format, self->priv->in_colorspace, colorimetry.matrix,
531+
colorimetry.range);
538532
}
539533
else
540534
{
@@ -548,9 +542,9 @@ static gboolean parse_input_caps(GstPispConvert *self, GstCaps *caps)
548542
self->priv->in_stride = GST_VIDEO_INFO_PLANE_STRIDE(&in_info, 0);
549543
self->priv->in_format = gst_format_to_pisp(GST_VIDEO_INFO_FORMAT(&in_info));
550544
self->priv->in_colorspace = colorimetry_to_pisp(&GST_VIDEO_INFO_COLORIMETRY(&in_info));
551-
GST_INFO_OBJECT(self, "Input format: pisp=%s, colorspace=%s (matrix=%d, range=%d)",
552-
self->priv->in_format, self->priv->in_colorspace,
553-
GST_VIDEO_INFO_COLORIMETRY(&in_info).matrix, GST_VIDEO_INFO_COLORIMETRY(&in_info).range);
545+
GST_INFO_OBJECT(self, "Input format: pisp=%s, colorspace=%s (matrix=%d, range=%d)", self->priv->in_format,
546+
self->priv->in_colorspace, GST_VIDEO_INFO_COLORIMETRY(&in_info).matrix,
547+
GST_VIDEO_INFO_COLORIMETRY(&in_info).range);
554548
}
555549

556550
if (!self->priv->in_colorspace)
@@ -661,14 +655,13 @@ static void add_video_meta(GstBuffer *buffer, const char *pisp_format, guint wid
661655
for (guint p = 0; p < n_planes; p++)
662656
{
663657
offsets[p] = offset;
664-
strides[p] = hw_stride * GST_VIDEO_INFO_PLANE_STRIDE(&vinfo, p) /
665-
GST_VIDEO_INFO_PLANE_STRIDE(&vinfo, 0);
666-
mem = gst_buffer_peek_memory (buffer, p);
658+
strides[p] = hw_stride * GST_VIDEO_INFO_PLANE_STRIDE(&vinfo, p) / GST_VIDEO_INFO_PLANE_STRIDE(&vinfo, 0);
659+
mem = gst_buffer_peek_memory(buffer, p);
667660
offset += mem->size;
668661
}
669662

670-
gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE, gst_fmt,
671-
width, height, n_planes, offsets, strides);
663+
gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE, gst_fmt, width, height, n_planes, offsets,
664+
strides);
672665
}
673666

674667
static void copy_planes(std::array<uint8_t *, 3> src, guint src_stride, std::array<uint8_t *, 3> dst, guint dst_stride,
@@ -887,19 +880,20 @@ static gboolean gst_pisp_convert_configure(GstPispConvert *self)
887880
self->priv->backend->SetOutputFormat(i, output_cfg[i]);
888881

889882
if ((g_str_equal(self->priv->out_format[i], "RGBX8888") ||
890-
g_str_equal(self->priv->out_format[i], "XRGB8888")) && !self->priv->variant->BackendRGB32Supported(0))
883+
g_str_equal(self->priv->out_format[i], "XRGB8888")) &&
884+
!self->priv->variant->BackendRGB32Supported(0))
891885
GST_WARNING_OBJECT(self, "pisp_be HW does not support 32-bit RGB output, the image will be corrupt.");
892886

893887
if (!self->priv->out_colorspace[i])
894888
self->priv->out_colorspace[i] = self->priv->in_colorspace;
895889

896-
global.rgb_enables |= configure_colour_conversion(self->priv->backend.get(),
897-
self->priv->in_format, self->priv->in_colorspace,
898-
self->priv->out_format[i], self->priv->out_colorspace[i], i);
890+
global.rgb_enables |= configure_colour_conversion(self->priv->backend.get(), self->priv->in_format,
891+
self->priv->in_colorspace, self->priv->out_format[i],
892+
self->priv->out_colorspace[i], i);
899893

900-
GST_INFO_OBJECT(self, "Output%d: %ux%u %s (stride: gst=%u hw=%u) colorspace %s", i, self->priv->out_width[i],
901-
self->priv->out_height[i], self->priv->out_format[i], self->priv->out_stride[i],
902-
self->priv->out_hw_stride[i], self->priv->out_colorspace[i]);
894+
GST_INFO_OBJECT(self, "Output%d: %ux%u %s (stride: gst=%u hw=%u) colorspace %s", i,
895+
self->priv->out_width[i], self->priv->out_height[i], self->priv->out_format[i],
896+
self->priv->out_stride[i], self->priv->out_hw_stride[i], self->priv->out_colorspace[i]);
903897
}
904898

905899
self->priv->backend->SetGlobal(global);
@@ -1141,8 +1135,8 @@ static GstFlowReturn gst_pisp_convert_chain(GstPad *pad [[maybe_unused]], GstObj
11411135
goto cleanup;
11421136
}
11431137

1144-
add_video_meta(outbuf[i], self->priv->out_format[i], self->priv->out_width[i],
1145-
self->priv->out_height[i], self->priv->out_hw_stride[i]);
1138+
add_video_meta(outbuf[i], self->priv->out_format[i], self->priv->out_width[i], self->priv->out_height[i],
1139+
self->priv->out_hw_stride[i]);
11461140

11471141
GST_DEBUG_OBJECT(self, "Using zero-copy output%d path", i);
11481142
}

0 commit comments

Comments
 (0)