GNOME Bugzilla – Bug 752367
dashdemux: adaptation set language could be better detected
Last modified: 2015-10-29 13:16:17 UTC
The gst_dash_demux_setup_all_streams function will try to set the language of the current stream. First it looks at the lang attribute in the AdaptationSet and if that is not found, it will look at the lang attribute in ContentComponent, but only if there is a single ContentComponent present. The standard says that if multiple components are present, their settings are specified using ContentComponent elements in the AdaptationSet (instead of embedding the settings directly in the AdaptationSet). Thus, if more than one component is present, the gst_dash_demux_setup_all_streams function will fail to set the language. The code /* Fallback to the language in ContentComponent node */ if (lang == NULL && g_list_length (adp_set->ContentComponents) == 1) { GstContentComponentNode *cc_node = adp_set->ContentComponents->data; lang = cc_node->lang; } should be rewritten to search the components list (regardless of size) and pick the first language found.
Created attachment 307404 [details] [review] proposed patch proposed patch
Review of attachment 307404 [details] [review]: ::: ext/dash/gstdashdemux.c @@ +507,3 @@ + for (gint j = 0; j < componentsCount; j++) { + GstContentComponentNode *cc_node = + g_list_nth_data (adp_set->ContentComponents, j); This could walk the list in O(N) instead of O(N^2)
Created attachment 310976 [details] [review] proposed patch improved list search to be done in O(n)
Review of attachment 310976 [details] [review]: ::: ext/dash/gstdashdemux.c @@ +542,3 @@ + for (GList * it = g_list_first (adp_set->ContentComponents); + it != g_list_last (adp_set->ContentComponents); + it = g_list_next (it)) { Doesn't that skip the last item in the list? Should it be: for (GList * it = g_list_first (adp_set->ContentComponents); it; it = g_list_next (it)) {
Review of attachment 310976 [details] [review]: ::: ext/dash/gstdashdemux.c @@ +542,3 @@ + for (GList * it = g_list_first (adp_set->ContentComponents); + it != g_list_last (adp_set->ContentComponents); + it = g_list_next (it)) { Also, declare the GList before the for ().. And you can skip g_list_first(), it does, nothing, just do for (it = adp_set->ContentComponents; it; it = it->next)
Created attachment 314387 [details] [review] proposed patch Florian's patch with the simpler list walking. g_list_last is also O(N) as this is not a double linked list. I'll be pushing that later today as I think that was the only comment pending.
commit e7745a57206b260f275e7a98cb9a8bf4fb8ce56d Author: Florin Apostol <florin.apostol@oregan.net> Date: Wed Sep 9 14:09:43 2015 +0100 dashdemux: improve detection of stream language Improved the detection of stream's language if the AdaptationSet contains more than 1 ContentComponent https://bugzilla.gnome.org/show_bug.cgi?id=752367