[PATCH 0/1] MR2546: winegstreamer: Create extra decodebin to decode compressed stream.
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/winegstreamer/wg_parser.c | 168 +++++++++++++++++++++++++++------ 1 file changed, 141 insertions(+), 27 deletions(-) diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 5bb824f4399..1a07ff6663e 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -57,8 +57,30 @@ typedef enum GST_DEBUG_CATEGORY(wine); #define GST_CAT_DEFAULT wine +static const char *raw_caps[] = +{ + "video/x-raw", + "audio/x-raw", + "text/x-raw", +}; + +static const char *compressed_caps[] = +{ + "video/x-wmv", + "video/x-h264", + "audio/x-mpeg", + "audio/x-wma", +}; + typedef BOOL (*init_gst_cb)(struct wg_parser *parser); +static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user); +static void pad_removed_cb(GstElement *element, GstPad *pad, gpointer user); +static gboolean autoplug_continue_cb(GstElement * decodebin, GstPad *pad, GstCaps * caps, gpointer user); +static GstAutoplugSelectResult autoplug_select_cb(GstElement *bin, GstPad *pad, + GstCaps *caps, GstElementFactory *fact, gpointer user); +static void no_more_pads_cb(GstElement *element, gpointer user); + struct wg_parser { init_gst_cb init_gst; @@ -104,7 +126,7 @@ 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; @@ -112,7 +134,7 @@ struct wg_parser_stream 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 +498,58 @@ static NTSTATUS wg_parser_stream_notify_qos(void *args) return S_OK; } +static bool 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 struct wg_parser_stream *get_stream_by_decodebin(struct wg_parser *parser, GstElement *decodebin) +{ + unsigned int i; + + for (i = 0; i < parser->stream_count; ++i) + { + if (decodebin == parser->streams[i]->decodebin) + return parser->streams[i]; + } + + return NULL; +} + +static gboolean autoplug_continue_cb(GstElement * decodebin, GstPad *pad, GstCaps * caps, gpointer user) +{ + struct wg_parser *parser = user; + const char *caps_name; + unsigned int i; + + caps_name = gst_structure_get_name(gst_caps_get_structure(caps, 0)); + + for (i = 0; i < ARRAY_SIZE(raw_caps); ++i) + { + if (!strcmp(caps_name, raw_caps[i])) + return FALSE; + } + + if (decodebin == parser->decodebin) + { + for (i = 0; i < ARRAY_SIZE(compressed_caps); ++i) + { + if (!strcmp(caps_name, compressed_caps[i])) + return FALSE; + } + } + + return TRUE; +} + static GstAutoplugSelectResult autoplug_select_cb(GstElement *bin, GstPad *pad, GstCaps *caps, GstElementFactory *fact, gpointer user) { @@ -505,11 +579,15 @@ static GstAutoplugSelectResult autoplug_select_cb(GstElement *bin, GstPad *pad, static void no_more_pads_cb(GstElement *element, gpointer user) { struct wg_parser *parser = user; + struct wg_parser_stream *stream; - GST_DEBUG("parser %p.", parser); + GST_DEBUG("parser %p, element %p", parser, element); pthread_mutex_lock(&parser->mutex); - parser->no_more_pads = true; + if ((stream = get_stream_by_decodebin(parser, element)) != NULL) + stream->no_more_pads = true; + else + parser->no_more_pads = true; pthread_mutex_unlock(&parser->mutex); pthread_cond_signal(&parser->init_cond); } @@ -766,6 +844,29 @@ GstElement *create_element(const char *name, const char *plugin_set) return element; } +GstElement *create_decodebin(struct wg_parser *parser) +{ + GstElement *decodebin; + + if (!(decodebin = create_element("decodebin", "base"))) + return NULL; + gst_bin_add(GST_BIN(parser->container), decodebin); + + if (parser->unlimited_buffering) + { + g_object_set(decodebin, "max-size-buffers", G_MAXUINT, NULL); + g_object_set(decodebin, "max-size-time", G_MAXUINT64, NULL); + g_object_set(decodebin, "max-size-bytes", G_MAXUINT, NULL); + } + g_signal_connect(decodebin, "pad-added", G_CALLBACK(pad_added_cb), parser); + g_signal_connect(decodebin, "pad-removed", G_CALLBACK(pad_removed_cb), parser); + g_signal_connect(decodebin, "autoplug-continue", G_CALLBACK(autoplug_continue_cb), parser); + g_signal_connect(decodebin, "autoplug-select", G_CALLBACK(autoplug_select_cb), parser); + g_signal_connect(decodebin, "no-more-pads", G_CALLBACK(no_more_pads_cb), parser); + + return decodebin; +} + static struct wg_parser_stream *create_stream(struct wg_parser *parser) { struct wg_parser_stream *stream, **new_array; @@ -782,6 +883,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); @@ -826,11 +928,12 @@ static void free_stream(struct wg_parser_stream *stream) static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) { + struct wg_parser_stream *stream = NULL; + GstPad *stream_decodebin_sink; struct wg_parser *parser = user; - struct wg_parser_stream *stream; const char *name; GstCaps *caps; - int ret; + int ret, i; GST_LOG("parser %p, element %p, pad %p.", parser, element, pad); @@ -840,8 +943,35 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) caps = gst_pad_query_caps(pad, NULL); name = gst_structure_get_name(gst_caps_get_structure(caps, 0)); - if (!(stream = create_stream(parser))) - goto out; + if (!(stream = get_stream_by_decodebin(parser, element))) + { + if (!(stream = create_stream(parser))) + goto out; + } + + if (element == parser->decodebin) + { + /* Create stream decodebin to decode compressed stream. */ + for (i = 0; i < ARRAY_SIZE(compressed_caps); ++i) + { + if (!strcmp(name, compressed_caps[i])) + { + stream->decodebin = create_decodebin(parser); + + pthread_mutex_lock(&parser->mutex); + stream->no_more_pads = false; + pthread_mutex_unlock(&parser->mutex); + gst_element_sync_state_with_parent(stream->decodebin); + + 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); + + goto out; + } + } + } if (!strcmp(name, "video/x-raw")) { @@ -1398,7 +1528,7 @@ static NTSTATUS wg_parser_connect(void *args) pthread_mutex_lock(&parser->mutex); - while (!parser->no_more_pads && !parser->error) + while (!no_more_pads(parser) && !parser->error) pthread_cond_wait(&parser->init_cond, &parser->mutex); if (parser->error) { @@ -1567,28 +1697,12 @@ static NTSTATUS wg_parser_disconnect(void *args) static BOOL decodebin_parser_init_gst(struct wg_parser *parser) { - GstElement *element; int ret; - if (!(element = create_element("decodebin", "base"))) + if (!(parser->decodebin = create_decodebin(parser))) return FALSE; - gst_bin_add(GST_BIN(parser->container), element); - parser->decodebin = element; - - if (parser->unlimited_buffering) - { - g_object_set(parser->decodebin, "max-size-buffers", G_MAXUINT, NULL); - g_object_set(parser->decodebin, "max-size-time", G_MAXUINT64, NULL); - g_object_set(parser->decodebin, "max-size-bytes", G_MAXUINT, NULL); - } - - 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-select", G_CALLBACK(autoplug_select_cb), parser); - g_signal_connect(element, "no-more-pads", G_CALLBACK(no_more_pads_cb), parser); - - parser->their_sink = gst_element_get_static_pad(element, "sink"); + parser->their_sink = gst_element_get_static_pad(parser->decodebin, "sink"); pthread_mutex_lock(&parser->mutex); parser->no_more_pads = false; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2546
Zebediah Figura (@zfigura) commented about dlls/winegstreamer/wg_parser.c:
+ return NULL; +} + +static gboolean autoplug_continue_cb(GstElement * decodebin, GstPad *pad, GstCaps * caps, gpointer user) +{ + struct wg_parser *parser = user; + const char *caps_name; + unsigned int i; + + caps_name = gst_structure_get_name(gst_caps_get_structure(caps, 0)); + + for (i = 0; i < ARRAY_SIZE(raw_caps); ++i) + { + if (!strcmp(caps_name, raw_caps[i])) + return FALSE; + } We shouldn't need this part; decodebin will detect that the caps are raw and stop anyway.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28528
Zebediah Figura (@zfigura) commented about dlls/winegstreamer/wg_parser.c:
+ + caps_name = gst_structure_get_name(gst_caps_get_structure(caps, 0)); + + for (i = 0; i < ARRAY_SIZE(raw_caps); ++i) + { + if (!strcmp(caps_name, raw_caps[i])) + return FALSE; + } + + if (decodebin == parser->decodebin) + { + for (i = 0; i < ARRAY_SIZE(compressed_caps); ++i) + { + if (!strcmp(caps_name, compressed_caps[i])) + return FALSE; + } I don't think we quite want to do it this way, rather we should probably just test if we can convert it to a wg_format. We'll need to do that anyway. Then we can just set a flag on the stream rather than later checking the array again.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28529
Zebediah Figura (@zfigura) commented about dlls/winegstreamer/wg_parser.c:
+ + if (parser->unlimited_buffering) + { + g_object_set(decodebin, "max-size-buffers", G_MAXUINT, NULL); + g_object_set(decodebin, "max-size-time", G_MAXUINT64, NULL); + g_object_set(decodebin, "max-size-bytes", G_MAXUINT, NULL); + } + g_signal_connect(decodebin, "pad-added", G_CALLBACK(pad_added_cb), parser); + g_signal_connect(decodebin, "pad-removed", G_CALLBACK(pad_removed_cb), parser); + g_signal_connect(decodebin, "autoplug-continue", G_CALLBACK(autoplug_continue_cb), parser); + g_signal_connect(decodebin, "autoplug-select", G_CALLBACK(autoplug_select_cb), parser); + g_signal_connect(decodebin, "no-more-pads", G_CALLBACK(no_more_pads_cb), parser); + + return decodebin; +} + I'm not sure I like using the same callbacks for both decodebins. pad-added does share a lot of code for postprocessing, but that could easily be a helper function. pad-removed isn't accounted for in this patch (and should be) and I think it needs to do something completely different for compressed and uncompressed elements. no-more-pads is small but can still afford to be duplicated.
Using separate callbacks for everything (except autoplug-select) strikes me as a bit architecturally clearer. It also has the benefit that get_stream_by_decodebin() is no longer needed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28530
On Fri Mar 31 19:35:56 2023 +0000, Zebediah Figura wrote:
I don't think we quite want to do it this way, rather we should probably just test if we can convert it to a wg_format. We'll need to do that anyway. Then we can just set a flag on the stream rather than later checking the array again. It seems that we can't set a flag to stream in autoplug_continue_cb(), because stream is created in pad_added_cb(), we can get the stream in autoplug_continue_cb() I think.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28601
pad-removed isn't accounted for in this patch (and should be) and I think it needs to do something completely different for compressed and uncompressed elements.
I'm not sure what else to do in extra decodebin's pad-removed, expect for the existing unlink stuff. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28609
On Mon Apr 3 02:06:54 2023 +0000, Ziqing Hui wrote:
It seems that we can't set a flag to stream in autoplug_continue_cb(), because stream is created in pad_added_cb(), we can not get the stream in autoplug_continue_cb() I think. And yeah, I do agree we should convert it to wg_format. I try to convert caps to wg_format in autoplug_continue_cb(), then run wmvcore tests. However, Gst complains about: `ERROR audio-info audio-info.c:302:gst_audio_info_from_caps: no rate property given`
I think the caps given in autoplug_continue_cb() is not complete, we cannot convert it to wg_format (without changing the existing wg_format_from_caps). So we should test the caps name in autoplug_continue_cb(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28610
On Mon Apr 3 03:45:50 2023 +0000, Ziqing Hui wrote:
I try to convert caps to wg_format in autoplug_continue_cb(), then run wmvcore tests. However, Gst complains: `ERROR audio-info audio-info.c:302:gst_audio_info_from_caps: no rate property given` I think the caps given in autoplug_continue_cb() is not complete, we cannot convert it to wg_format (without changing the existing wg_format_from_caps). So we should test the caps name in autoplug_continue_cb(). 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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2546#note_28628
participants (3)
-
Zebediah Figura (＠zfigura) -
Ziqing Hui
-
Ziqing Hui (＠zhui)