Thread (13 messages) 13 messages, 3 authors, 2014-09-23

Re: [PATCH 1/5] video: move mediabus format definition to a more standard place

flat view

From: Boris BREZILLON <hidden>
Date: 2014-09-23 16:09:31
Also in: dri-devel, linux-media, lkml

Hi Guennadi,

On Tue, 23 Sep 2014 14:33:20 +0200 (CEST)
Guennadi Liakhovetski [off-list ref] wrote:
Hi Boris,

On Tue, 22 Jul 2014, Boris BREZILLON wrote:
quoted
Rename mediabus formats and move the enum into a separate header file so
that it can be used by DRM/KMS subsystem without any reference to the V4L2
subsystem.

Old V4L2_MBUS_FMT_ definitions are now macros that points to VIDEO_BUS_FMT_
definitions.

Signed-off-by: Boris BREZILLON <redacted>
In principle I find this a good idea, certainly it's good to reuse code. 
Just wondering, wouldn't it be better instead of adding those defines to 
define a macro like

#define VIDEO_BUS_TO_MBUS(x)	V4L2_MBUS_ ## x = VIDEO_BUS_ ## x

and then do

enum v4l2_mbus_pixelcode {
	VIDEO_BUS_TO_MBUS(FIXED),
	VIDEO_BUS_TO_MBUS(RGB444_2X8_PADHI_BE),
	...
};

? I'm not very strong on this, I just think an enum is nicer than a bunch 
of defines and this way copy-paste errors are less likely, but if you or 
others strongly disagree - I won't insist :)
I'd say it might be a good solution if we enforce new users (including
user space users) to use video_bus_format enum values instead of
v4l2_mbus_pixelcode ones. But if we keep adding new values to this enum
I'd say this approach is less readable than having the full names
(V4L2_MBUS_XXX) expressed, because users will still have to use those
full names.

Anyway, I can still replace this macro list by an enum:

enum v4l2_mbus_pixelcode {
	V4L2_MBUS_FIXED = VIDEO_BUS_FIXED,
	V4L2_MBUS_RGB444_2X8_PADHI_BE = VIDEO_BUS_RGB444_2X8_PADHI_BE,
	...
};

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help