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 750390 - tests: dashdemux: add unit tests for checking the parsing of MPD element
tests: dashdemux: add unit tests for checking the parsing of MPD element
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.5.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-06-04 11:12 UTC by Florin Apostol
Modified: 2015-06-08 16:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch containing the proposed unit tests (16.20 KB, patch)
2015-06-04 11:12 UTC, Florin Apostol
none Details | Review
Updated patch based on feedback from review (16.22 KB, patch)
2015-06-05 16:28 UTC, Florin Apostol
none Details | Review

Description Florin Apostol 2015-06-04 11:12:32 UTC
Created attachment 304572 [details] [review]
Patch containing the proposed unit tests

Create a dash test target and add unit tests that check the functions from ext/dash/gstmpdparser.c file.

These tests check the compliance of the MPD parser in the dashdemux element against the DASH specification.
Comment 1 Thiago Sousa Santos 2015-06-05 15:06:00 UTC
Review of attachment 304572 [details] [review]:

This is nice!

One remarks:
Could you rename the test to be dash_mpd instead of dash? The hlsdemux_m3u8 test follows the same pattern.

Some other minor stuff annotated in the review,

::: tests/check/elements/dash.c
@@ +1,1 @@
+/* GStreamer unit test for MPEG-DASH

Alignment issue here, just add one extra space

@@ +43,3 @@
+
+  assert_equals_int (ret, TRUE);
+  // check that unset elements with default values are properly configured

We avoid using C++ style comments in gstreamer code

@@ +136,3 @@
+  assert_equals_int64 (mpdclient->mpd_node->maxSubsegmentDuration,
+      (((((gint64) 0 * 365 + 1 * 30 + 2) * 24 + 12) * 60 + 10) * 60 +
+          20) * 1000 + 500);

Perhaps it would make sense to refactor all this math into a macro that gives a name to the operation for easier readability

@@ +411,3 @@
+GST_END_TEST;
+
+// create a test suite containing all dash testcases

Another c++ style comment here, there are a few more in the code as well.
Comment 2 Florin Apostol 2015-06-05 16:28:22 UTC
Created attachment 304659 [details] [review]
Updated patch based on feedback from review

Replaced c++ comments wit c style comments.
Renamed dash.c file to dash_mpd.c
Added function to convert duration to ms.
Comment 3 Thiago Sousa Santos 2015-06-08 16:32:46 UTC
Merged with a minor change to make the test binary be called 'dash_mpd' instead of 'dash'. Thanks for the patch

commit 8b18be5bfb7544915cb55b031a7b77787f64e3a2
Author: Florin Apostol <florin.apostol@oregan.net>
Date:   Fri Jun 5 17:20:20 2015 +0100

    tests: dashdemux: add unit tests for checking the parsing of MPD element
    
    Create a dash test target and add unit tests that check the parsing of
    attributes of the MPD element and the following child elements:
     - baseURL
     - program information
     - location
     - metrics
    
    These tests check the compliance of parts of the MPD parser in the
    dashdemux element against the DASH specification [1].
    
        [1] http://standards.iso.org/ittf/PubliclyAvailableStandards/c065274_ISO_IEC_23009-1_2014_Electronic_inserts.zip
            http://standards.iso.org/ittf/PubliclyAvailableStandards/c065274_ISO_IEC_23009-1_2014.zip
    
    https://bugzilla.gnome.org/show_bug.cgi?id=750390
Comment 4 Thiago Sousa Santos 2015-06-08 16:36:18 UTC
Comment on attachment 304659 [details] [review]
Updated patch based on feedback from review

Marking as obsolete as the upstream version is slightly different.