[Bug 750390] tests: dashdemux: add unit tests for checking the parsing of MPD element

GStreamer (GNOME Bugzilla) bugzilla at gnome.org
Fri Jun 5 08:06:00 PDT 2015


https://bugzilla.gnome.org/show_bug.cgi?id=750390

Thiago Sousa Santos <thiagossantos at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #304572|none                        |reviewed
             status|                            |

--- Comment #1 from Thiago Sousa Santos <thiagossantos at gmail.com> ---
Review of attachment 304572:
 --> (https://bugzilla.gnome.org/review?bug=750390&attachment=304572)

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.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.


More information about the gstreamer-bugs mailing list