On 2/11/22 17:46, Rémi Bernon wrote:
On 2/12/22 00:19, Zebediah Figura wrote:
On 2/11/22 03:36, Rémi Bernon wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51931 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52391 Signed-off-by: Rémi Bernon rbernon@codeweavers.com
This would serve a simpler purpose than the full-fledged wg_parser, and be used by synchronous transforms only (at least there's no thread on the Wine side), which simply work on the push then pull loop model.
Do you by chance have a complete branch somewhere that I can look at or mess with? I'd in particular like to see just how much code is identical, or explicitly different, between the parser and transform objects.
The wg_transform name is chosen to be generic and possibly cover both decoder and encoders (and possibly converters), but maybe it's better to keep it specific for now and only design it for decoders. This would simplify the format question in the next patch.
I suspect it makes more sense for it to be a generic transform object. From what I've seen GStreamer doesn't really discriminate between encoders, decoders, and other transforms, and I don't think that winegstreamer should either.
I have three branches on top of each other, wip/wmadev/v1, wip/h264dec/v1, and wip/aacdec/v1 in https://github.com/rbernon/wine, although the last one is not polished at all and full of wip patches for tests.
Probably the most interesting things are in the wmadec and h264dec patches. As far as I could see (although I didn't test it much) the aacdec didn't really require anything except the format mappings.
Thanks, that's about what I was looking for.
I only ended up looking at the WMA branch, but here are my thoughts:
* There's surprisingly even less code in there than I thought, so I'm inclined to think that a separate wg_transform is a good idea. That said:
* wg_transform is built without an equivalent of wg_parser_get_preferred_format() and all of the relevant support code. That gives me some pause. It doesn't give me a *lot* of pause, because the main point of wg_transform is to handle those applications that try to create *specific* decoders, and we probably want those specific decoders to report the same exact formats as they do on Windows. There are a few examples I can think of of applications that would be able to make use of a generic decoder, but if we have enough specific decoders already there may be no point anyway.
* wg_transform is not designed to handle one-to-many elements and cuts out all of the relevant code. I think this is fine, ultimately; the only real example of that I can think of is the case of a demuxer that supports push mode, and if we ever need to support that (which we probably won't) it *probably* makes more sense to use wg_parser anyway.
* The post-processing logic is kind of duplicated. Would it make sense to separate that into a helper function and then use it for both wg_parser and wg_transform?
* sink_chain_cb is kind of redone. To be fair, we don't care about EOS or segment events. On the other hand, I'm not thrilled about the way we pull outputs in wg_parser, and we probably want to make it look more like what you have for wg_transform. [In specific I suspect we want to generate our own segment events for DirectShow rather than using GStreamer's, and once that's done we should make EOS a return value, and preëmptively grab sample buffers...] I can probably live with having two different implementations for a while, but ultimately I think we want to avoid that, at least in the unixlib API.
* I think we want to handle GST_MESSAGE_ERROR and GST_MESSAGE_WARNING. We can't use bus_handler_cb() directly (since we don't want to handle GST_MESSAGE_DURATION_CHANGED) but we could probably add a helper for those two at least.
* We might want QoS logic. That's probably not that impactful in terms of "is wg_transform worth it", though, and it doesn't need to be done immediately anyway...
* More of a case of "if you don't want to do it then I will sooner or later", but if we're going to have two different objects I'd prefer to move the common code (which ends up mostly being all of the wg_format conversion) to a third file, just for maintainability's sake.