GNOME Bugzilla – Bug 619292
[API] add chunk writer
Last modified: 2018-11-03 12:13:58 UTC
Bytewriting typically involves nested structures with sizes recorded in the bytestream (e.g. avi chunks, qt atoms, ebml elements, jpeg markers, etc). Additional proposed API aids in tracking nested structures positions and sizes, which could be used in any of the above cases, rather than coming up with yet another home made on in each case separately. Evidently, some variations on the proposed are possible, e.g. using a separate (inherited) type for this (GstByteWriterStack or so).
Created attachment 161643 [details] [review] Add mark stacking API
Review of attachment 161643 [details] [review]: ::: libs/gst/base/gstbytewriter.h @@ +47,3 @@ + + /*< private >*/ + GArray *marks; Sadly I don't think we can do this, because we allow allocation of byte writers on the stack and "forgot" to add padding to these structs. Guess we need a GstChunkWriter class or so (which could just keep track of the chunks and return/init a byte writer to write into as chunk? Another thing we could have done if we had padding - allow for a 'parent' byte writer..)
It took me a bit of thinking about the API to get an idea of what it's supposed to do. I'm still not quite sure though. Could you explain what you want the API to be used for and give a small pseudo-code example with your proposed API? Because I have some other ideas on how to do something like that, but I'm not sure they fit your use case.
I found bug 619293 and its attached use case. I think I have a solution that works fine (I think it works even better) without new API. I should probably elaborate on why I don't like APIs like this: My problem is that it's not a byte writer's job to care about nesting. A byte writer's job is to care about writing bytes to a data buffer and that should be it. There's nothing against sophisticated APIs for doing this kind of job, but keeping the API focussed on this one job is an important thing IMO.
I was wondering from the start whether this belonged to a "pure" bytewriter, as so indicated by an alternative as a separate type in the original report. Given that, and the padding problem, a more appropriate one would be a separate (e.g.) GstChunkWriter that would be associated to a GstByteWriter (rather than inherited or intertwined API).
I've always been wanting API like this myself, but was never quite sure how it should look like (and too lazy). Would be useful for avimux, matroskamux, qtmux, asfmux, wavenc, etc. An associated byte writer instead of inheritance sounds like a good plan to me, that way we get the 'backed by a single memory region' thing automatically.
If I were to do something like this, I'd have an API that hands you a new bytewriter object that writes a subpart of the parent bytewriter. But this is not really what you want for the chunked files use case - all you want here is write the size of a chunk later. And I think rewind(), put_uint32(), back_to_end() - style API is very suitable for that. And you can support that very well with offsets, which are basically the same thing as marks. A different API would probably make sense if you wanted to reserve a fixed-size section of more than 4 bytes and write them later or maybe even interchangably with the rest of the byte writer (dunno, maybe an index?), where it'd be nice to be able to create a fixed size byte writer ala subsection_writer = gst_byte_writer_new_for_section (parent_writer, num_bytes); but that is hard to do with the current bytewriter API (due to resizes invalidating the pointer) and I don't think we have any use cases for it.
Created attachment 161919 [details] [review] Add GstChunkWriter So here is another stab at some stack marking API, this time in a separate GstChunkWriter helper associated with a GstByteWriter. In any case, it is not exactly humongous API.
Comment on attachment 161919 [details] [review] Add GstChunkWriter Looks good to me, but could use a unit test, maybe an initialization macro to init it on the stack, and a use in a plugin somewhere. After that I'm fine with having this pushed.
I always thought this would be handy, but I don't think it should be pushed before there's a user for it too.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/14.