[PATCH 0/39] MR5615: Draft: winegstreamer: Convert media type to GstCaps directly instead of through an internal representation.
This saves ~1500 LoC, and at the same time allows mapping arbitrary long codec data back and forth GstCaps, which will be required to support arbitrary compressed formats. We can then for instance expose unknown codecs as Wine-specific subtypes, expose generic decoders and pass the backend GstCaps serialized in the codec data. It makes the code simpler by unifying all frontend mappings and only requiring mapping between GstCaps and a unified set of formats. Mapping between IMFMediaType and the AM_MEDIA_TYPE formats is done using the standard MF functions and are tested and validated in mfplat tests. -- This merge request has too many patches to be relayed via email. Please visit the URL below to see the contents of the merge request. https://gitlab.winehq.org/wine/wine/-/merge_requests/5615
How much of code deletion comes from switching to a two-step mapping between DirectShow and Media Foundation formats, versus changing the format we pass over the wire? Are the DirectShow <-> Media Foundation conversions perfectly lossless in both directions, for all compressed and uncompressed types? Are they correct (i.e. even if they're lossless, how much do we have to fix up the types afterward)? If not, what differences and pitfalls are there exactly? (Also, why rename wg_format to wg_media_type?) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5615#note_70556
How much of code deletion comes from switching to a two-step mapping between DirectShow and Media Foundation formats, versus changing the format we pass over the wire?
I don't know? Most of it?
Are the DirectShow \<-\> Media Foundation conversions perfectly lossless in both directions, for all compressed and uncompressed types? Are they correct (i.e. even if they're lossless, how much do we have to fix up the types afterward)? If not, what differences and pitfalls are there exactly?
Media foundation can express more information than DirectShow formats, which is also why MFVIDEOFORMAT is used internally. There's some fixups to be done with MPEG formats, because they are converted to some specific MF attributes (instead of MF_MT_USER_DATA), but it's not a big deal to fixup. Other than that there's AM_MEDIA_TYPE fields that sometimes differ from what the component expose, but we had some fixups already depending on the frontends, so it doesn't change much.
(Also, why rename wg_format to wg_media_type?)
It just makes the transition easier. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5615#note_70565
Anton Baskanov (@baskanov) commented about dlls/winegstreamer/quartz_parser.c:
return hr; }
static HRESULT avi_splitter_source_get_media_type(struct parser_source *pin, unsigned int index, AM_MEDIA_TYPE *mt) { - struct wg_format format; + HRESULT hr;
if (index > 0) return VFW_S_NO_MORE_ITEMS; - wg_parser_stream_get_current_format(pin->wg_stream, &format); - if (!amt_from_wg_format(mt, &format, false)) - return E_OUTOFMEMORY; - return S_OK; + if (SUCCEEDED(hr = wg_parser_stream_get_current_type_quartz(pin->wg_stream, mt)))
Compressed media types returned by `wg_parser_stream_get_current_type_quartz()` have `biSizeImage` set to 0. This will cause `IMemAllocator::Commit()` to fail. `amt_from_wg_format()` estimated `biSizeImage` at least for Cinepak, so we should probably do something similar here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5615#note_70755
On Fri May 17 08:41:46 2024 +0000, Anton Baskanov wrote:
Compressed media types returned by `wg_parser_stream_get_current_type_quartz()` have `biSizeImage` set to 0. This will cause `IMemAllocator::Commit()` to fail. `amt_from_wg_format()` estimated `biSizeImage` at least for Cinepak, so we should probably do something similar here. Yeah, there is something similar in mpeg_splitter_source_get_media_type already.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5615#note_70756
How much of code deletion comes from switching to a two-step mapping between DirectShow and Media Foundation formats, versus changing the format we pass over the wire?
I don't know? Most of it?
Most of it results from the two-step mapping, or from changing the format we pass over the wire? I mean, it matters. There's an inherent tradeoff in both cases (besides code churn of course). Using a two-step mapping risks losing information (at least if we use the builtin methods, and there's no point if we're not going to do that), even if that's not something we notice now it might come up later. Similarly wg_format was something that was specifically meant as a compact, neutral, well-defined representation, with no extra information, that's easy to pass over the wire. These two things can be changed independently, and knowing how much of a benefit we get from each helps to evaluate whether each change is something we want.
(Also, why rename wg_format to wg_media_type?)
It just makes the transition easier.
How does renaming a struct make anything easier? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5615#note_70972
I think you're just making things more complicated than they are. The wg_format struct is just another ad-hoc internal struct, and a subset of native representations. Choosing another internal representation, and in this case MFVIDEOFORMAT / WAVEFORMATEX should be safe and enough to achieve the exact same thing.
How does renaming a struct make anything easier?
It's not renamed, it's a different struct to allow to gradually transition the fronted. Does it even matter? It better matches native terminology anyway, where `format` is the representation, and `media type` is the additional metadata around it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5615#note_71066
This merge request was closed by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5615
participants (3)
-
Anton Baskanov (＠baskanov) -
Elizabeth Figura (＠zfigura)
-
Rémi Bernon