On Tue Apr 11 13:26:36 2023 +0000, eric pouech wrote:
in fact there might be two issues to take care of (covering also your next comment): - various decoders are now exposed to potentially negative heights, which some of them are not prepared for (eg. h264 or dmo) - orientation when MF_MT_DEFAULT_STRIDE isn't specified in MediaType (color_converter and videoconverter may provide behave differently) so I wonder which design is better: - hide completly the flip information to decoders (passing abs(height)) and handle in wg_transform only - expose the flip request to decoders in case they are able to handle it? (and fallback to flip in wg_transform if they don't handle it) looks like we maybe need some more tests for various stride values Fwiw I don't think native allows negative `MF_MT_DEFAULT_STRIDE` for anything else than RGB formats, and we shouldn't either. H264 decoder doesn't support RGB format output, but WMV decoder does and I think we should check how it behaves wrt frame orientation (eventually adding a test).
We should probably keep the flipping decision to wg_transform, almost like it's done. Actually more like it was done initially as you'll need to keep a `struct wg_format input_format` and `GstElement *videoflip` in the `struct wg_transform` to use them in `wg_transform_set_output_format`. I'd use a helper like this, and simply compare `wg_format_is_flipped(input) != wg_format_is_flipped(output)` to decide which value to set the videoflip method to: ```C static bool wg_format_is_flipped(const struct wg_format *format) { return format->major_type == WG_MAJOR_TYPE_VIDEO && format->u.video.height < 0; } ``` Then, regarding how `video.height` is set, this is purely a matter of media type translation. I think it's even visible when you test the MF conversion functions to translate a `IMFMediaType` to the various legacy representation like `AM_MEDIA_TYPE`, through `IMFMediaType_GetRepresentation` for instance. We don't have these conversions functions, at the moment and we probably don't need them for this, but the `wg_format` translation should probably follow them. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2471#note_29639