On Wed Sep 28 21:16:15 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list:
On 9/28/22 16:13, Zebediah Figura wrote: > On 9/27/22 03:32, Rémi Bernon wrote: >> +/* List all supported raw video formats, with their metadata. >> + * >> + * Each element should be X(wg, guid, depth, type), with: >> + * wg: suffix of the GStreamer constant, and enum wg_video_format >> value. >> + * guid: suffix of the corresponding MEDIASUBTYPE / MFVideoFormat >> guid. >> + * depth: the number of bits of each video sample. >> + * type: whether the video format is RGB or YUV. >> + * >> + */ >> +#define FOR_EACH_WG_VIDEO_FORMAT(F) \ >> + F(BGRA, ARGB32, 32, RGB) \ >> + F(BGRx, RGB32, 32, RGB) \ >> + F(BGR, RGB24, 24, RGB) \ >> + F(RGB15, RGB555, 16, RGB) \ >> + F(RGB16, RGB565, 16, RGB) \ >> + F(AYUV, AYUV, 32, YUV) \ >> + F(I420, I420, 12, YUV) \ >> + F(NV12, NV12, 12, YUV) \ >> + F(UYVY, UYVY, 16, YUV) \ >> + F(YUY2, YUY2, 16, YUV) \ >> + F(YV12, YV12, 12, YUV) \ >> + F(YVYU, YVYU, 16, YUV) \ >> + >> +enum wg_video_format >> +{ >> + WG_VIDEO_FORMAT_UNKNOWN = 0, >> +#define X(x, g, d, t) WG_VIDEO_FORMAT_ ## x, >> + FOR_EACH_WG_VIDEO_FORMAT(X) >> +#undef X >> +}; > > I don't hate this, but I'm also less than thrilled about it. > > One of the nice things about spelling things out explicitly is that it's > easy for someone unfamiliar with the code to grep for constants to see > how they're converted. > > This also puts some burden on the reader to mentally preprocess the code > first. It's probably not a big deal, since it's not difficult to > understand what it's doing, but I'm not really sure it's worthwhile in > this case. > > FWIW, I think the other patches in this series are fine (namely 2, 3, 4, 7). I don't easily see patch 1/7 as being an improvement if not required for this patch, but the renames seem like an improvement.
As we're using separate major types for encoded formats I think CINEPAK has nothing to do with the raw video formats, even though it only requires the same set of properties as raw formats. For instance I don't think signed height makes any sense, the data doesn't have a stride.