4 Oct
2022
4 Oct
'22
7:06 p.m.
On 9/30/22 04:47, Rémi Bernon (@rbernon) wrote:
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.
Sure, but it's mostly been a matter of convenience. Also perhaps clarity, in terms of avoiding fields being ignored for certain formats—but that's also not something I'm especially afraid of, especially in the case of cinepak where all of the fields can be shared, signed height notwithstanding.