[PATCH 0/1] MR1945: Draft: winegstreamer: Return correct format in stream_props_GetMediaType.
This patch doesn't work now. Zebediah, I think I need your help. My goal is to get the original stream format in `stream_props_GetMediaType()`. For example, if I use wmreader to open a WMV3 stream, then this function should return WMV3 format as its result. I expect preferred_format to hold the format I need. But test shows that preferred_format is always RGB24 even if I open a WMV stream. I found that preferred_format is written in `wg_parser.c: sink_event_cb(): case GST_EVENT_CAPS`. So it means that the caps here is "video/x-raw", not "video/x-wmv" as I expected. So how could I get the stream format I want when I open a compressed stream using wmreader? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1945
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/quartz_parser.c | 52 +++++++++++++++++++++++++++++- dlls/winegstreamer/wg_format.c | 44 ++++++++++++++++++++++++- dlls/winegstreamer/wm_reader.c | 10 +++++- 3 files changed, 103 insertions(+), 3 deletions(-) diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c index 5561b106327..448868b0b2c 100644 --- a/dlls/winegstreamer/quartz_parser.c +++ b/dlls/winegstreamer/quartz_parser.c @@ -538,6 +538,54 @@ static bool amt_from_wg_format_video_cinepak(AM_MEDIA_TYPE *mt, const struct wg_ return true; } +static bool amt_from_wg_format_video_wmv(AM_MEDIA_TYPE *mt, const struct wg_format *format) +{ + VIDEOINFO *video_format; + uint32_t frame_time; + const GUID *subtype; + + if (!(video_format = CoTaskMemAlloc(sizeof(*video_format)))) + return false; + + switch (format->u.video_wmv.version) + { + case 1: + subtype = &MEDIASUBTYPE_WMV1; + break; + case 2: + subtype = &MEDIASUBTYPE_WMV2; + break; + case 3: + subtype = &MEDIASUBTYPE_WMV3; + break; + default: + WARN("Invalid wmvversion %u.\n", format->u.video_wmv.version); + return false; + } + + mt->majortype = MEDIATYPE_Video; + mt->subtype = *subtype; + mt->bTemporalCompression = TRUE; + mt->lSampleSize = 1; + mt->formattype = FORMAT_VideoInfo; + mt->cbFormat = sizeof(VIDEOINFOHEADER); + mt->pbFormat = (BYTE *)video_format; + + memset(video_format, 0, sizeof(*video_format)); + SetRect(&video_format->rcSource, 0, 0, format->u.video_wmv.width, format->u.video_wmv.height); + video_format->rcTarget = video_format->rcSource; + if ((frame_time = MulDiv(10000000, format->u.video_wmv.fps_d, format->u.video_wmv.fps_n)) != -1) + video_format->AvgTimePerFrame = frame_time; + video_format->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); + video_format->bmiHeader.biWidth = format->u.video_wmv.width; + video_format->bmiHeader.biHeight = format->u.video_wmv.height; + video_format->bmiHeader.biPlanes = 1; + video_format->bmiHeader.biBitCount = 24; + video_format->bmiHeader.biCompression = mt->subtype.Data1; + + return true; +} + bool amt_from_wg_format(AM_MEDIA_TYPE *mt, const struct wg_format *format, bool wm) { memset(mt, 0, sizeof(*mt)); @@ -547,7 +595,6 @@ bool amt_from_wg_format(AM_MEDIA_TYPE *mt, const struct wg_format *format, bool case WG_MAJOR_TYPE_AUDIO_MPEG4: case WG_MAJOR_TYPE_AUDIO_WMA: case WG_MAJOR_TYPE_VIDEO_H264: - case WG_MAJOR_TYPE_VIDEO_WMV: FIXME("Format %u not implemented!\n", format->major_type); /* fallthrough */ case WG_MAJOR_TYPE_UNKNOWN: @@ -564,6 +611,9 @@ bool amt_from_wg_format(AM_MEDIA_TYPE *mt, const struct wg_format *format, bool case WG_MAJOR_TYPE_VIDEO_CINEPAK: return amt_from_wg_format_video_cinepak(mt, format); + + case WG_MAJOR_TYPE_VIDEO_WMV: + return amt_from_wg_format_video_wmv(mt, format); } assert(0); diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c index a7876977d6c..1dac5085781 100644 --- a/dlls/winegstreamer/wg_format.c +++ b/dlls/winegstreamer/wg_format.c @@ -232,6 +232,40 @@ static void wg_format_from_caps_video_cinepak(struct wg_format *format, const Gs format->u.video_cinepak.fps_d = fps_d; } +static void wg_format_from_caps_video_wmv(struct wg_format *format, const GstCaps *caps) +{ + const GstStructure *structure = gst_caps_get_structure(caps, 0); + gint width, height, fps_n, fps_d, version; + + if (!gst_structure_get_int(structure, "width", &width)) + { + GST_WARNING("Missing \"width\" value."); + return; + } + if (!gst_structure_get_int(structure, "height", &height)) + { + GST_WARNING("Missing \"height\" value."); + return; + } + if (!gst_structure_get_int(structure, "wmvversion", &version)) + { + GST_WARNING("Missing \"wmvversion\" value."); + return; + } + if (!gst_structure_get_fraction(structure, "framerate", &fps_n, &fps_d)) + { + fps_n = 0; + fps_d = 1; + } + + format->major_type = WG_MAJOR_TYPE_VIDEO_WMV; + format->u.video_wmv.width = width; + format->u.video_wmv.height = height; + format->u.video_wmv.version = version; + format->u.video_wmv.fps_n = fps_n; + format->u.video_wmv.fps_d = fps_d; +} + void wg_format_from_caps(struct wg_format *format, const GstCaps *caps) { const GstStructure *structure = gst_caps_get_structure(caps, 0); @@ -261,6 +295,10 @@ void wg_format_from_caps(struct wg_format *format, const GstCaps *caps) { wg_format_from_caps_video_cinepak(format, caps); } + else if (!strcmp(name, "video/x-wmv")) + { + wg_format_from_caps_video_wmv(format, caps); + } else { gchar *str = gst_caps_to_string(caps); @@ -611,7 +649,6 @@ bool wg_format_compare(const struct wg_format *a, const struct wg_format *b) case WG_MAJOR_TYPE_AUDIO_MPEG4: case WG_MAJOR_TYPE_AUDIO_WMA: case WG_MAJOR_TYPE_VIDEO_H264: - case WG_MAJOR_TYPE_VIDEO_WMV: GST_FIXME("Format %u not implemented!", a->major_type); /* fallthrough */ case WG_MAJOR_TYPE_UNKNOWN: @@ -633,6 +670,11 @@ bool wg_format_compare(const struct wg_format *a, const struct wg_format *b) /* Do not compare FPS. */ return a->u.video_cinepak.width == b->u.video_cinepak.width && a->u.video_cinepak.height == b->u.video_cinepak.height; + + case WG_MAJOR_TYPE_VIDEO_WMV: + return a->u.video_wmv.width == b->u.video_wmv.width + && a->u.video_wmv.height == b->u.video_wmv.height + && a->u.video_wmv.version == b->u.video_wmv.version; } assert(0); diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c index 18b0e8a90bc..278a8dae995 100644 --- a/dlls/winegstreamer/wm_reader.c +++ b/dlls/winegstreamer/wm_reader.c @@ -546,12 +546,20 @@ static HRESULT WINAPI stream_props_GetMediaType(IWMMediaProps *iface, WM_MEDIA_T struct stream_config *config = impl_from_IWMMediaProps(iface); const DWORD req_size = *size; AM_MEDIA_TYPE stream_mt; + struct wg_format format; TRACE("iface %p, mt %p, size %p.\n", iface, mt, size); - if (!amt_from_wg_format(&stream_mt, &config->stream->format, true)) + wg_parser_stream_get_preferred_format(config->stream->wg_stream, &format); + if (!amt_from_wg_format(&stream_mt, &format, true)) return E_OUTOFMEMORY; + /* Debug usage, will be removed when patch is submitted to upstream. + * According to tests, for WMV1 video, preferred_format is still RGB24, which is expected to be WMV1 here. + * So, how to get stream real format when using wmreader to read a compressed stream ? + * For example, I'd like get WMV3 format if I use wmreader to read a WMV3 stream. */ + WINE_MESSAGE("==== format.major_type %d, format.u.video.format %d.\n", format.major_type, format.u.video.format); + *size = sizeof(stream_mt) + stream_mt.cbFormat; if (!mt) return S_OK; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1945
decodebin has a "caps" property that controls when decoding ends. What we should probably do is, on creation, grab those caps, append (or prepend?) the right caps (in this case I think video/x-wmv) to that and set it. Since this is configurable we'll want this to be an argument to wg_parser creation. I think we want to do this one set of caps at a time, as necessary. For efficiency and simplicity we still want to prefer decoding the entire pipeline. Note that one interesting aspect of this is that in wmvcore this behaviour is configurable. I don't *think* there's a way to signal decodebin to "re-decode" its streams, or at least I couldn't easily find one, so I think we'll need to drop and recreate the wg_parser when that property is changed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1945#note_20774
Side note, the wg_format translation should probably be a separate commit when this is done. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1945#note_20775
On Mon Mar 6 07:34:41 2023 +0000, Zebediah Figura wrote:
decodebin has a "caps" property that controls when decoding ends. What we should probably do is, on creation, grab those caps, append (or prepend?) the right caps (in this case I think video/x-wmv) to that and set it. Since this is configurable we'll want this to be an argument to wg_parser creation. I think we want to do this one set of caps at a time, as necessary. For efficiency and simplicity we still want to prefer decoding the entire pipeline. Note that one interesting aspect of this is that in wmvcore this behaviour is configurable. I don't *think* there's a way to signal decodebin to "re-decode" its streams, or at least I couldn't easily find one, so I think we'll need to drop and recreate the wg_parser when that property is changed. I think we need to do something that is very similar to wm_reader.c: reader_SetOutputProps(). Here is what reader_SetOutputProps() do:
``` reader_SetOutputProps() { ... wg_parser_stream_enable(stream->wg_stream); ... wg_parser_stream_seek(reader->streams[0].wg_stream, 1.0, reader->start_time, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning); } ``` See: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/winegstreamer/wm_read... In wg_parser_stream_enable(), it calls gst_pad_push_event(gst_event_new_reconfigure()) to reconfigure the pipeline. Document of gst_event_new_reconfigure():
gst_event_new_reconfigure:
Create a new reconfigure event. The purpose of the reconfigure event is to travel upstream and make elements renegotiate their caps or reconfigure their buffer pools. This is useful when changing properties on elements or changing the topology of the pipeline.
Returns: (transfer full): a new #GstEvent
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1945#note_26179
On Mon Mar 6 07:34:41 2023 +0000, Ziqing Hui wrote:
I think we need to do something that is very similar to wm_reader.c: reader_SetOutputProps(). Here is what reader_SetOutputProps() do: ``` reader_SetOutputProps() { ... wg_parser_stream_enable(stream->wg_stream); ... wg_parser_stream_seek(reader->streams[0].wg_stream, 1.0, reader->start_time, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning); } ``` See: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/winegstreamer/wm_read... In wg_parser_stream_enable(), it calls gst_pad_push_event(gst_event_new_reconfigure()) to reconfigure the pipeline. Document of gst_event_new_reconfigure():
gst_event_new_reconfigure:
Create a new reconfigure event. The purpose of the reconfigure event is to travel upstream and make elements renegotiate their caps or reconfigure their buffer pools. This is useful when changing properties on elements or changing the topology of the pipeline.
Returns: (transfer full): a new #GstEvent If we could do "reconfigure and seek" like reader_SetOutputProps(). Then we don't need to recreate wg_parser. What do you think?
@rbernon @zfigura -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1945#note_26180
If we could do "reconfigure and seek" like reader_SetOutputProps(). Then we don't need to recreate wg_parser. What do you think?
I don't believe that reconfigure is sufficient, no. My understanding is that it will not trigger decodebin to remove elements from the pipeline; it will only ask the existing elements to change what they are outputting. For a decoder, or a transcoder like videoconvert, this is sufficient to change the format from e.g. one RGB format to another, but I doubt it will be sufficient to change whether a decoder is in fact decoding or not. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1945#note_26229
I don't believe that reconfigure is sufficient, no. My understanding is that it will not trigger decodebin to remove elements from the pipeline; it will only ask the existing elements to change what they are outputting. For a decoder, or a transcoder like videoconvert, this is sufficient to change the format from e.g. one RGB format to another, but I doubt it will be sufficient to change whether a decoder is in fact decoding or not.
OK, so we still need to recreate the pipeline. Do we recreate the whole wg_parser or only recreate decodebin pipeline? Another question: if we assume that the caps configure only happens during setting up the reader, before decoding anything, and ignore the mid-stream situation for now. Do we still need to do the recreate things when caps configure happens? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1945#note_26363
OK, so we still need to recreate the pipeline. Do we recreate the whole wg_parser or only recreate decodebin pipeline?
Without having tried it, the former sounds easier; it requires less infrastructure, and it's what we already do when changing the format in transforms.
Another question: if we assume that the caps configure only happens during setting up the reader, before decoding anything, and ignore the mid-stream situation for now. Do we still need to do the recreate things when caps configure happens?
We currently create the parser when the stream is opened, and we can't create it any later (since we need to query things like stream count and stream formats). On the other hand, toggling decoding is done on the stream interfaces, which has to be done after the stream is opened. So we definitely need to recreate the parser, one way or another. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1945#note_26432
This merge request was closed by Ziqing Hui. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1945
participants (3)
-
Zebediah Figura (＠zfigura) -
Ziqing Hui
-
Ziqing Hui (＠zhui)