[PATCH v2 0/2] MR2546: winegstreamer: Create extra decodebin to decode compressed stream.
-- v2: wingstreamer: Create extra decodebin to decode compressed stream. winegstreamer: Implement wg_format_from_caps_video_wmv. https://gitlab.winehq.org/wine/wine/-/merge_requests/2546
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/wg_format.c | 57 ++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c index ac21b0af94f..1850972cdbc 100644 --- a/dlls/winegstreamer/wg_format.c +++ b/dlls/winegstreamer/wg_format.c @@ -232,6 +232,59 @@ 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, wmv_version = 0; + gchar format_buffer[4] = {'W','M','V','0'}; + enum wg_wmv_video_format wmv_format; + const gchar *wmv_format_str = NULL; + + 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 (!(wmv_format_str = gst_structure_get_string(structure, "format"))) + { + if (!gst_structure_get_int(structure, "wmvversion", &wmv_version)) + GST_WARNING("Unable to get WMV format."); + format_buffer[3] += wmv_version; + wmv_format_str = format_buffer; + } + if (!strcmp(wmv_format_str, "WMV1")) + wmv_format = WG_WMV_VIDEO_FORMAT_WMV1; + else if (!strcmp(wmv_format_str, "WMV2")) + wmv_format = WG_WMV_VIDEO_FORMAT_WMV2; + else if (!strcmp(wmv_format_str, "WMV3")) + wmv_format = WG_WMV_VIDEO_FORMAT_WMV3; + else if (!strcmp(wmv_format_str, "WMVA")) + wmv_format = WG_WMV_VIDEO_FORMAT_WMVA; + else if (!strcmp(wmv_format_str, "WVC1")) + wmv_format = WG_WMV_VIDEO_FORMAT_WVC1; + else + wmv_format = WG_WMV_VIDEO_FORMAT_UNKNOWN; + + 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.format = wmv_format; + 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 +314,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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2546
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/wg_parser.c | 207 ++++++++++++++++++++++++++++----- 1 file changed, 178 insertions(+), 29 deletions(-) diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 5bb824f4399..eb996ecd6ca 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -104,15 +104,15 @@ struct wg_parser_stream uint32_t number; GstPad *their_src, *post_sink, *post_src, *my_sink; - GstElement *flip; + GstElement *flip, *decodebin; GstSegment segment; - struct wg_format preferred_format, current_format; + struct wg_format preferred_format, current_format, stream_format; pthread_cond_t event_cond, event_empty_cond; GstBuffer *buffer; GstMapInfo map_info; - bool flushing, eos, enabled, has_caps, has_tags, has_buffer; + bool flushing, eos, enabled, has_caps, has_tags, has_buffer, no_more_pads; uint64_t duration; gchar *tags[WG_PARSER_TAG_COUNT]; @@ -476,6 +476,29 @@ static NTSTATUS wg_parser_stream_notify_qos(void *args) return S_OK; } +static bool parser_no_more_pads(struct wg_parser *parser) +{ + unsigned int i; + + for (i = 0; i < parser->stream_count; ++i) + { + if (!parser->streams[i]->no_more_pads) + return false; + } + + return parser->no_more_pads; +} + +static gboolean autoplug_continue_cb(GstElement * decodebin, GstPad *pad, GstCaps * caps, gpointer user) +{ + const char *name = gst_structure_get_name(gst_caps_get_structure(caps, 0)); + + if (!strcmp(name, "video/x-wmv")) + return false; + + return true; +} + static GstAutoplugSelectResult autoplug_select_cb(GstElement *bin, GstPad *pad, GstCaps *caps, GstElementFactory *fact, gpointer user) { @@ -782,6 +805,7 @@ static struct wg_parser_stream *create_stream(struct wg_parser *parser) stream->parser = parser; stream->number = parser->stream_count; + stream->no_more_pads = true; stream->current_format.major_type = WG_MAJOR_TYPE_UNKNOWN; pthread_cond_init(&stream->event_cond, NULL); pthread_cond_init(&stream->event_empty_cond, NULL); @@ -824,24 +848,15 @@ static void free_stream(struct wg_parser_stream *stream) free(stream); } -static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) +static bool stream_create_post_processing_elements(struct wg_parser_stream *stream) { - struct wg_parser *parser = user; - struct wg_parser_stream *stream; + struct wg_parser *parser = stream->parser; const char *name; GstCaps *caps; - int ret; - GST_LOG("parser %p, element %p, pad %p.", parser, element, pad); - - if (gst_pad_is_linked(pad)) - return; - - caps = gst_pad_query_caps(pad, NULL); + caps = gst_pad_query_caps(stream->their_src, NULL); name = gst_structure_get_name(gst_caps_get_structure(caps, 0)); - - if (!(stream = create_stream(parser))) - goto out; + gst_caps_unref(caps); if (!strcmp(name, "video/x-raw")) { @@ -850,22 +865,22 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) /* DirectShow can express interlaced video, but downstream filters can't * necessarily consume it. In particular, the video renderer can't. */ if (!(deinterlace = create_element("deinterlace", "good"))) - goto out; + return false; /* decodebin considers many YUV formats to be "raw", but some quartz * filters can't handle those. Also, videoflip can't handle all "raw" * formats either. Add a videoconvert to swap color spaces. */ if (!(vconv = create_element("videoconvert", "base"))) - goto out; + return false; /* GStreamer outputs RGB video top-down, but DirectShow expects bottom-up. */ if (!(flip = create_element("videoflip", "good"))) - goto out; + return false; /* videoflip does not support 15 and 16-bit RGB so add a second videoconvert * to do the final conversion. */ if (!(vconv2 = create_element("videoconvert", "base"))) - goto out; + return false; /* The bin takes ownership of these elements. */ gst_bin_add(GST_BIN(parser->container), deinterlace); @@ -894,7 +909,7 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) * 64-bit formats either. Add an audioconvert to allow changing bit * depth and channel count. */ if (!(convert = create_element("audioconvert", "base"))) - goto out; + return false; gst_bin_add(GST_BIN(parser->container), convert); gst_element_sync_state_with_parent(convert); @@ -903,15 +918,22 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) stream->post_src = gst_element_get_static_pad(convert, "src"); } + return true; +} + +static bool stream_link_elements(struct wg_parser_stream *stream) +{ + int ret; + if (stream->post_sink) { - if ((ret = gst_pad_link(pad, stream->post_sink)) < 0) + if ((ret = gst_pad_link(stream->their_src, stream->post_sink)) < 0) { GST_ERROR("Failed to link decodebin source pad to post-processing elements, error %s.", gst_pad_link_get_name(ret)); gst_object_unref(stream->post_sink); stream->post_sink = NULL; - goto out; + return false; } if ((ret = gst_pad_link(stream->post_src, stream->my_sink)) < 0) @@ -922,20 +944,146 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) stream->post_src = NULL; gst_object_unref(stream->post_sink); stream->post_sink = NULL; - goto out; + return false; } } - else if ((ret = gst_pad_link(pad, stream->my_sink)) < 0) + else if ((ret = gst_pad_link(stream->their_src, stream->my_sink)) < 0) { GST_ERROR("Failed to link decodebin source pad to our sink pad, error %s.", gst_pad_link_get_name(ret)); - goto out; + return false; } - gst_pad_set_active(stream->my_sink, 1); + return true; +} + +static void stream_decodebin_no_more_pads_cb(GstElement *element, gpointer user) +{ + struct wg_parser_stream *stream = user; + struct wg_parser *parser = stream->parser; + + GST_DEBUG("stream %p, parser %p, element %p.", stream, parser, element); + + pthread_mutex_lock(&parser->mutex); + stream->no_more_pads = true; + pthread_mutex_unlock(&parser->mutex); + pthread_cond_signal(&parser->init_cond); +} + +static void stream_decodebin_pad_added_cb(GstElement *element, GstPad *pad, gpointer user) +{ + struct wg_parser_stream *stream = user; + struct wg_parser *parser = stream->parser; + + GST_LOG("stream %p, parser %p, element %p, pad %p.", stream, parser, element, pad); + + if (gst_pad_is_linked(pad)) + return; + gst_object_ref(stream->their_src = pad); -out: + if (!stream_create_post_processing_elements(stream)) + return; + if (!stream_link_elements(stream)) + return; + gst_pad_set_active(stream->my_sink, 1); +} + +static void stream_decodebin_pad_removed_cb(GstElement *element, GstPad *pad, gpointer user) +{ + struct wg_parser_stream *stream = user; + struct wg_parser *parser = stream->parser; + + GST_LOG("stream %p, parser %p, element %p, pad %p.", stream, parser, element, pad); + + if (stream->their_src == pad) + { + if (stream->post_sink) + gst_pad_unlink(stream->their_src, stream->post_sink); + else + gst_pad_unlink(stream->their_src, stream->my_sink); + + gst_object_unref(stream->their_src); + stream->their_src = NULL; + return; + } +} + +static bool stream_decodebin_create(struct wg_parser_stream *stream) +{ + struct wg_parser *parser = stream->parser; + + GST_LOG("stream %p, parser %p.", stream, parser); + + if (!(stream->decodebin = create_element("decodebin", "base"))) + return false; + gst_bin_add(GST_BIN(parser->container), stream->decodebin); + + if (parser->unlimited_buffering) + { + g_object_set(stream->decodebin, "max-size-buffers", G_MAXUINT, NULL); + g_object_set(stream->decodebin, "max-size-time", G_MAXUINT64, NULL); + g_object_set(stream->decodebin, "max-size-bytes", G_MAXUINT, NULL); + } + g_signal_connect(stream->decodebin, "pad-added", G_CALLBACK(stream_decodebin_pad_added_cb), stream); + g_signal_connect(stream->decodebin, "pad-removed", G_CALLBACK(stream_decodebin_pad_removed_cb), stream); + g_signal_connect(stream->decodebin, "autoplug-select", G_CALLBACK(autoplug_select_cb), stream); + g_signal_connect(stream->decodebin, "no-more-pads", G_CALLBACK(stream_decodebin_no_more_pads_cb), stream); + + pthread_mutex_lock(&parser->mutex); + stream->no_more_pads = false; + pthread_mutex_unlock(&parser->mutex); + gst_element_sync_state_with_parent(stream->decodebin); + + GST_LOG("Created stream decodebin %p for %u.", stream->decodebin, stream->number); + + return true; +} + +static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) +{ + struct wg_parser_stream *stream; + struct wg_parser *parser = user; + GstPad *stream_decodebin_sink; + GstCaps *caps; + int ret; + + GST_LOG("parser %p, element %p, pad %p.", parser, element, pad); + + if (gst_pad_is_linked(pad)) + return; + + if (!(stream = create_stream(parser))) + return; + + caps = gst_pad_query_caps(pad, NULL); + wg_format_from_caps(&stream->stream_format, caps); gst_caps_unref(caps); + + /* For compressed stream, create an extra decodebin to decode it. */ + if (stream->stream_format.major_type != WG_MAJOR_TYPE_UNKNOWN + && stream->stream_format.major_type != WG_MAJOR_TYPE_VIDEO + && stream->stream_format.major_type != WG_MAJOR_TYPE_AUDIO) + { + if (!stream_decodebin_create(stream)) + { + GST_ERROR("Unable to create decodebin for stream %u.", stream->number); + return; + } + + stream_decodebin_sink = gst_element_get_static_pad(stream->decodebin, "sink"); + if ((ret = gst_pad_link(pad, stream_decodebin_sink)) < 0) + GST_ERROR("Failed to link pads, error %d.", ret); + gst_object_unref(stream_decodebin_sink); + + return; + } + + gst_object_ref(stream->their_src = pad); + if (!stream_create_post_processing_elements(stream)) + return; + if (!stream_link_elements(stream)) + return; + gst_pad_set_active(stream->my_sink, 1); } static void pad_removed_cb(GstElement *element, GstPad *pad, gpointer user) @@ -1398,7 +1546,7 @@ static NTSTATUS wg_parser_connect(void *args) pthread_mutex_lock(&parser->mutex); - while (!parser->no_more_pads && !parser->error) + while (!parser_no_more_pads(parser) && !parser->error) pthread_cond_wait(&parser->init_cond, &parser->mutex); if (parser->error) { @@ -1585,6 +1733,7 @@ static BOOL decodebin_parser_init_gst(struct wg_parser *parser) g_signal_connect(element, "pad-added", G_CALLBACK(pad_added_cb), parser); g_signal_connect(element, "pad-removed", G_CALLBACK(pad_removed_cb), parser); + g_signal_connect(element, "autoplug-continue", G_CALLBACK(autoplug_continue_cb), parser); g_signal_connect(element, "autoplug-select", G_CALLBACK(autoplug_select_cb), parser); g_signal_connect(element, "no-more-pads", G_CALLBACK(no_more_pads_cb), parser); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2546
If I run wmvcore tests for V2, no failure for the test. However, as I said before, during the tests, gstreamer will complain abount: `ERROR audio-info audio-info.c:302:gst_audio_info_from_caps: no rate property given` This error comes the wg_format_from_caps() call inside pad_added_cb(). Also, if I call wg_format_from_caps() in autoplug_continue_cb(), same error will happen. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28645
I found that `gst_audio_info_from_caps error` also happens in pad_added_cb(). The caps from the pad given in pad_added_cb() is also missing rate property.
That's interesting. Is that PCM audio? What are the full caps? I assume it's triggered by the test.wmv in wmvcore? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28736
On Mon Apr 3 08:33:18 2023 +0000, Ziqing Hui wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/2546/diffs?diff_id=40282&start_sha=5a3dd701d483b5849504a3a5c343f17091ec8a7c#d19138de09a66cf3bcf4e698b2f5c81e9daf14a2_869_792) For the demuxer pad-removed I believe we at least need to unlink the compressed pad (the one that's being removed) from the downstream decodebin. We don't need to release the decodebin because the larger bin has ownership, though.
In that case I don't think that the stream pad-removed actually needs to do anything, if unlinking the final source pad is handled by the demuxer pad-removed. It's maybe a bit awkward that we lose symmetry that way, but still probably better. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28737
On Mon Apr 3 18:59:37 2023 +0000, Zebediah Figura wrote:
I found that `gst_audio_info_from_caps error` also happens in pad_added_cb(). The caps from the pad given in pad_added_cb() is also missing rate property. That's interesting. Is that PCM audio? What are the full caps? I assume it's triggered by the test.wmv in wmvcore? Yep, it's triggered by the WMV in wmvcore. The full caps of the audio is:
``` audio/x-raw format: F32LE layout: non-interleaved ``` The full caps of the video stream from the same WMV file is: ``` video/x-wmv wmvversion: 1 width: 64 height: 48 pixel-aspect-ratio: 1/1 format: WMV1 ``` Both caps are printed in pad_added_cb(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28793
On Tue Apr 4 01:18:05 2023 +0000, Ziqing Hui wrote:
Yep, it's triggered by the WMV in wmvcore. The full caps of the audio is: ``` audio/x-raw format: F32LE layout: non-interleaved ``` The full caps of the video stream from the same WMV file is: ``` video/x-wmv wmvversion: 1 width: 64 height: 48 pixel-aspect-ratio: 1/1 format: WMV1 ``` Both caps are printed in pad_added_cb(). Maybe we could implement our own wg_format_from_caps_audio() rather than using gst_audio_info_from_caps() directly? Like the other wg_format_from_caps_*() functions, we warn when find missing fileds instead of error.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28794
Maybe we could implement our own wg_format_from_caps_audio() rather than using gst_audio_info_from_caps() directly? Like the other wg_format_from_caps\_\*() functions, we warn when find missing fileds instead of error.
I don't think that solves the problem. If we're missing the sample rate, that's kind of a serious problem; we kind of need the sample rate in order to report it downstream. Does the stream ever give more specific caps? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28797
On Tue Apr 4 02:07:04 2023 +0000, Zebediah Figura wrote:
Maybe we could implement our own wg_format_from_caps_audio() rather than using gst_audio_info_from_caps() directly? Like the other wg_format_from_caps\_\*() functions, we warn when find missing fileds instead of error. I don't think that solves the problem. If we're missing the sample rate, that's kind of a serious problem; we kind of need the sample rate in order to report it downstream. Does the stream ever give more specific caps? I found that the original audio stream is actually wma. Caps of the wma stream is:
``` audio/x-wma wmaversion: 1 bitrate: 128000 depth: 16 rate: 44100 channels: 1 block_align: 743 codec_data: 00000100 ``` It loses some caps field after our reader decode it into audio/x-raw. I don't know why. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28800
On Tue Apr 4 02:46:59 2023 +0000, Ziqing Hui wrote:
I found that the original audio stream is actually wma. Caps of the wma stream is: ``` audio/x-wma wmaversion: 1 bitrate: 128000 depth: 16 rate: 44100 channels: 1 block_align: 743 codec_data: 00000100 ``` (This caps is printed in autoplug_continue_cb()) It loses some caps field after our reader decode it into audio/x-raw. I don't know why. Here is the test patch. [test.patch](/uploads/bc1b2351c474a413e67c5c663996b326/test.patch) The full caps info printed by this patch when running wmvcore tests is: ``` autoplug_continue_cb: video/x-ms-asf autoplug_select_cb: using ASF Demuxer autoplug_continue_cb: video/x-wmv autoplug_continue_cb: wmvversion: 1 autoplug_continue_cb: width: 64 autoplug_continue_cb: height: 48 autoplug_continue_cb: pixel-aspect-ratio: 1/1 autoplug_continue_cb: format: WMV1 autoplug_continue_cb: audio/x-wma autoplug_continue_cb: wmaversion: 1 autoplug_continue_cb: bitrate: 128000 autoplug_continue_cb: depth: 16 autoplug_continue_cb: rate: 44100 autoplug_continue_cb: channels: 1 autoplug_continue_cb: block_align: 743 autoplug_continue_cb: codec_data: 00000100 autoplug_select_cb: using libav Windows Media Audio 1 decoder autoplug_continue_cb: audio/x-raw autoplug_continue_cb: format: F32LE autoplug_continue_cb: layout: non-interleaved pad_added_cb: video/x-wmv pad_added_cb: wmvversion: 1 pad_added_cb: width: 64 pad_added_cb: height: 48 pad_added_cb: pixel-aspect-ratio: 1/1 pad_added_cb: format: WMV1 pad_added_cb: audio/x-raw pad_added_cb: format: F32LE pad_added_cb: layout: non-interleaved ``` Here's the result of `gst-inspect-1.0 avdec_wmav1`:
``` ... Pad Templates: SINK template: 'sink' Availability: Always Capabilities: audio/x-wma wmaversion: 1 block_align: [ 0, 2147483647 ] bitrate: [ 0, 2147483647 ] SRC template: 'src' Availability: Always Capabilities: audio/x-raw format: F32LE layout: non-interleaved ... ``` The src pad template here is identical to what we see in autoplug_continue_cb(). Also, see: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/g...
From the decodebin source, we can see that the caps argument passed to autoplug_continue comes from the plugin pad template. And avdec_wmav1 plugin src pad template doesn't have rate, which leads to the missing rate in autoplug_continue.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_29132
On Thu Apr 6 08:45:42 2023 +0000, Ziqing Hui wrote:
Here's the result of `gst-inspect-1.0 avdec_wmav1`: ``` ... Pad Templates: SINK template: 'sink' Availability: Always Capabilities: audio/x-wma wmaversion: 1 block_align: [ 0, 2147483647 ] bitrate: [ 0, 2147483647 ]
SRC template: 'src' Availability: Always Capabilities: audio/x-raw format: F32LE layout: non-interleaved ... ``` The src pad template here is identical to what we see in autoplug_continue_cb(). Also, see: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/g... From the decodebin source, we can see that the caps argument passed to autoplug_continue comes from the plugin pad template. And avdec_wmav1 plugin src pad template doesn't have rate, which leads to the missing rate in autoplug_continue. If I run wmvcore test with this patch: [test.patch](/uploads/c6da0600f0e5ce11a9e4a29f831e0bcb/test.patch)
The result will be: ``` autoplug_continue_cb: video/x-ms-asf Using "ASF Demuxer". autoplug_continue_cb: video/x-wmv autoplug_continue_cb: wmvversion: 1 autoplug_continue_cb: width: 64 autoplug_continue_cb: height: 48 autoplug_continue_cb: pixel-aspect-ratio: 1/1 autoplug_continue_cb: format: WMV1 Using "libav Windows Media Video 7 decoder". autoplug_continue_cb: audio/x-wma autoplug_continue_cb: wmaversion: 1 autoplug_continue_cb: bitrate: 128000 autoplug_continue_cb: depth: 16 autoplug_continue_cb: rate: 44100 autoplug_continue_cb: channels: 1 autoplug_continue_cb: block_align: 743 autoplug_continue_cb: codec_data: 00000100 Using "libav Windows Media Audio 1 decoder". autoplug_continue_cb: audio/x-raw autoplug_continue_cb: format: F32LE autoplug_continue_cb: layout: non-interleaved sink_event_cb: GST_EVENT_CAPS: audio/x-raw sink_event_cb: GST_EVENT_CAPS: format: F32LE sink_event_cb: GST_EVENT_CAPS: layout: non-interleaved sink_event_cb: GST_EVENT_CAPS: rate: 44100 sink_event_cb: GST_EVENT_CAPS: channels: 1 sink_event_cb: GST_EVENT_CAPS: video/x-raw sink_event_cb: GST_EVENT_CAPS: format: I420 sink_event_cb: GST_EVENT_CAPS: width: 64 sink_event_cb: GST_EVENT_CAPS: height: 48 sink_event_cb: GST_EVENT_CAPS: interlace-mode: progressive sink_event_cb: GST_EVENT_CAPS: pixel-aspect-ratio: 1/1 sink_event_cb: GST_EVENT_CAPS: framerate: 25/1 sink_event_cb: GST_EVENT_CAPS: audio/x-raw sink_event_cb: GST_EVENT_CAPS: format: S16LE sink_event_cb: GST_EVENT_CAPS: layout: interleaved sink_event_cb: GST_EVENT_CAPS: rate: 44100 sink_event_cb: GST_EVENT_CAPS: channels: 1 sink_event_cb: GST_EVENT_CAPS: channel-mask: 0x0000000000000004 sink_event_cb: GST_EVENT_CAPS: video/x-raw sink_event_cb: GST_EVENT_CAPS: format: BGR sink_event_cb: GST_EVENT_CAPS: width: 64 sink_event_cb: GST_EVENT_CAPS: height: 48 sink_event_cb: GST_EVENT_CAPS: interlace-mode: progressive sink_event_cb: GST_EVENT_CAPS: pixel-aspect-ratio: 1/1 sink_event_cb: GST_EVENT_CAPS: framerate: 25/1 ```
I don't think that solves the problem. If we're missing the sample rate, that's kind of a serious problem; we kind of need the sample rate in order to report it downstream. Does the stream ever give more specific caps?
We can see that althought rate is missing in autoplug_continue, it is given in GST_EVENT_CAPS. So I think it's OK to use an our own version of wg_format_from_caps_audio(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_29133
From the decodebin source, we can see that the caps argument passed to autoplug_continue comes from the plugin pad template. And avdec_wmav1 plugin src pad template doesn't have rate, which leads to the missing rate in autoplug_continue.
I'm not quite sure where you're getting that, but as far as I can tell, get_pad_caps() does query the current caps, not the template caps. I think CAPS queries might fall back to the template if nothing else is set, but I don't remember. The problem seems more subtle: ffmpegauddec actually sends multiple caps events before sending any data, where the second caps event is more specific than the first. For some reason (either consistent timing, or that second caps is always sent before the video caps are fixed) this second caps is always sent before pads are exposed via pad-added, so Wine will always see the more specific caps. This is a bit annoying—yet another case where GStreamer's "reactive" model doesn't fit the "proactive" model needed for any win32 multimedia API. It does mean that wg_parser_stream_get_codec_format() is not going to be perfectly reliable, but ultimately few things involving GStreamer are. Probably we should have wg_parser_stream_get_codec_format() only return the format from autoplug-continue if it was actually compressed, and just alias wg_parser_stream_get_preferred_format() otherwise, to account for this. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_29197
I'm not quite sure where you're getting that,
I do believe I'm right about this. I'm getting it from: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/g... The decodebin source code shows: ```c analyse_new_pad() { ... for (tmps = gst_element_factory_get_static_pad_templates (factory); tmps && !dontuse; tmps = tmps->next) { GstStaticPadTemplate *st = (GstStaticPadTemplate *) tmps->data; if (st->direction != GST_PAD_SRC) continue; tcaps = gst_static_pad_template_get_caps (st); apcontinue = TRUE; /* Emit autoplug-continue to see if the caps are considered to be raw caps */ g_signal_emit (G_OBJECT (dbin), gst_decode_bin_signals[SIGNAL_AUTOPLUG_CONTINUE], 0, dpad, tcaps, &apcontinue); /* If autoplug-continue returns TRUE and the caps are not final, and * we haven't found any way to output finals yet, don't use them */ if (are_final_caps (dbin, tcaps)) found_finals = TRUE; else if (apcontinue && !found_finals) dontuse = TRUE; gst_caps_unref (tcaps); } ... } ``` It does not use get_pad_caps(), instead, it uses gst_static_pad_template_get_caps() to get caps (the `tcaps` virable), and then pass it to autoplug_continue. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_29267
Probably we should have wg_parser_stream_get_codec_format() only return the format from autoplug-continue if it was actually compressed, and just alias wg_parser_stream_get_preferred_format() otherwise, to account for this.
I got 2 questions: 1. We record codec format(aka stream format) in `struct wg_parser_stream`, but this struct is created in pad_added_cb() by create_stream(). So we can't get the stream struct in autoplug_continue_cb() because we don't have wg_parser_stream when autoplug_continue_cb() is called. We should record the codec format when pad_added_cb() is called. Am I right? 2. if 1. is correct, do we still need to call wg_fromat_from_caps() in autoplug_continue_cb(),? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_29268
participants (3)
-
Zebediah Figura (＠zfigura) -
Ziqing Hui
-
Ziqing Hui (＠zhui)