GNOME Bugzilla – Bug 506132
Review of changes in video/video.h
Last modified: 2008-01-14 18:21:02 UTC
Please comment on recent additions to video.h.
These might also be nice to have: gboolean gst_video_format_is_packed (GstVideoFormat format); int gst_video_format_get_pixel_offset (GstVideoFormat format, int component, int width, int height, int x, int y); The latter would be a utility function for plane_offset + y*foo + x*bar. (not sure about the name of the latter, maybe _get_offset(), or _get_position_offset() would be better).
Random addendum: it's sure nice to finally have all this stuff in libgstvideo, but it's all very tedious to use. The function names are very long, so are the video format constants, and you have to do lots of calls to get the strides/offsets etc. The code just doesn't look nice. I'd prefer a function to which I can just pass format/width/height/pixeldatapointer and which returns a struct with all the stuff needed (offset by pixeldatapointer).
I tried it both ways, and concluded this was better. Using a struct ends up with longer, less readable lines: pixelstride = format->components[i].pixelstride; pixelstride = gst_format_get_pixel_stride (format, i);
Why not just format->strides[i] ?
The difference between using functions vs. structures at this level is pretty much a matter of style. However, my reasoning here is mostly: - Functions are easier to extend: just add another function. Structures, you have to worry about ABI issues. - Functions are simple and obviously named. IMO, it's much harder for a user to understand the fields in a structure than the return value of a function. This is especially true when some of the structure fields are irrelevant for certain formats. - With structures, you have to worry about refcounting/freeing - Functions map very easily into other languages. Structures somewhat, but it's more difficult and takes effort.
All good arguments :) I read the code on the weekend, I'm happy with the API - can we close the bug, or would someone like step up to add the other API Tim suggested (It might not make today's pre-release, but there's another window for one on Thursday)?
I'm fine with it as it is. Dave's argument for functions makes sense. Convenience functions can always be added later, same goes for _is_packed(). I don't have an urgent need for either right now.
OK, ta. Marking as closed.