After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 752367 - dashdemux: adaptation set language could be better detected
dashdemux: adaptation set language could be better detected
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-07-14 11:27 UTC by Florin Apostol
Modified: 2015-10-29 13:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (1.36 KB, patch)
2015-07-14 11:46 UTC, Florin Apostol
none Details | Review
proposed patch (1.35 KB, patch)
2015-09-09 13:11 UTC, Florin Apostol
none Details | Review
proposed patch (1.27 KB, patch)
2015-10-29 12:16 UTC, Vincent Penquerc'h
committed Details | Review

Description Florin Apostol 2015-07-14 11:27:03 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.
Comment 1 Florin Apostol 2015-07-14 11:46:29 UTC
Created attachment 307404 [details] [review]
proposed patch

proposed patch
Comment 2 Vincent Penquerc'h 2015-09-09 10:52:09 UTC
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)
Comment 3 Florin Apostol 2015-09-09 13:11:37 UTC
Created attachment 310976 [details] [review]
proposed patch

improved list search to be done in O(n)
Comment 4 A Ashley 2015-10-02 09:02:51 UTC
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)) {
Comment 5 Olivier Crête 2015-10-02 14:05:56 UTC
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)
Comment 6 Vincent Penquerc'h 2015-10-29 12:16:29 UTC
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.
Comment 7 Vincent Penquerc'h 2015-10-29 13:15:59 UTC
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