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.