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 350340 - CMML test cases and small fixes
CMML test cases and small fixes
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 0.10.5
Assigned To: Wim Taymans
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-08-07 20:40 UTC by Alessandro Decina
Modified: 2006-08-25 09:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fixes (2.75 KB, patch)
2006-08-07 20:42 UTC, Alessandro Decina
needs-work Details | Review
tests (34.71 KB, patch)
2006-08-07 20:44 UTC, Alessandro Decina
needs-work Details | Review
fixes (3.99 KB, patch)
2006-08-24 21:54 UTC, Alessandro Decina
committed Details | Review
tests (35.38 KB, patch)
2006-08-24 21:57 UTC, Alessandro Decina
committed Details | Review

Description Alessandro Decina 2006-08-07 20:40:46 UTC
I'm attaching two patches, one that adds test cases to cmmlenc and cmmldec, the other which fixes some small bugs in gstcmmlenc.c and gstcmmlutils.c.
I can't commit myself as I don't have cvs access, so please review and in case commit them.
Comment 1 Alessandro Decina 2006-08-07 20:42:39 UTC
Created attachment 70423 [details] [review]
fixes

2006-08-07  Alessandro Decina <alessandro@nnva.org>

	* ext/annodex/gstcmmlenc.c (gst_cmml_enc_parse_tag_clip),
	(gst_cmml_enc_push_clip): Check if clip->start_time is valid before
	adding the clip to the track list.
	(gst_cmml_enc_change_state): Reset enc->preamble going from PAUSED to READY.

	* ext/annodex/gstcmmlutils.c (gst_cmml_clock_time_from_npt): Parse the
	seconds field of the npt-sec time format using %llu rather than %d and check
	that the value scaled by GST_SECOND doesn't overflow.
	(gst_cmml_clock_time_to_granule): Use guint64(s) to represent the keyindex and
	keyoffset fields of a granulepos.
	(gst_cmml_track_list_has_clip): Lookup a clip's track with clip->track rather
	than clip->id which makes no sense.
	Identify a clip by its track and start time and not its xml id.
Comment 2 Alessandro Decina 2006-08-07 20:44:51 UTC
Created attachment 70425 [details] [review]
tests
Comment 3 Wim Taymans 2006-08-23 11:16:44 UTC
With the both patches applied, the test fails like this:

Running suite(s): cmmldec
100%: Checks: 6, Failures: 0, Errors: 0
PASS: elements/cmmldec
Running suite(s): cmmlenc0:00:00.276794000 30131 0x8050a08 ERROR              cmmlenc gstcmmlenc.c:513:gst_cmml_enc_parse_tag_clip:<cmmlenc> previous clip start time > current clip (clip-2) start time

80%: Checks: 5, Failures: 1, Errors: 0
elements/cmmlenc.c:420:F:general:test_time_limits: 'flow' (GST_FLOW_ERROR) is not equal to 'GST_FLOW_OK' (GST_FLOW_OK)
FAIL: elements/cmmlenc
Comment 4 Alessandro Decina 2006-08-24 21:54:53 UTC
Created attachment 71548 [details] [review]
fixes

I should really remember to test my code on x86.
Looks like ((guint64) 1 << 64) is undefined but it gives the correct value with gcc on linux-ppc32. This is fixed now.
Comment 5 Alessandro Decina 2006-08-24 21:57:34 UTC
Created attachment 71550 [details] [review]
tests
Comment 6 Wim Taymans 2006-08-25 09:44:18 UTC
Fixed in CVS.

I did some small changes to the cmmlenc element, mainly wrong usage of GST_FLOW_UNEXPECTED and posting an ERROR message when we did not generate the error.

        Patch by: Alessandro Decina <alessandro at nnva dot org>

        * ext/annodex/gstannodex.c: (gst_annodex_granule_to_time):
        Do some extra sanity checks.
        Fixes #350340.

        * ext/annodex/gstcmmlenc.c: (gst_cmml_enc_change_state),
        (gst_cmml_enc_parse_tag_head), (gst_cmml_enc_parse_tag_clip),
        (gst_cmml_enc_push_clip), (gst_cmml_enc_push):
        Check if clip->start_time is valid before adding the clip to the
        track list.
        Reset enc->preamble going from PAUSED to READY.
        Don't use GST_FLOW_UNEXPECTED for wrong usage of the element, it is
        only used for EOS.
        Only post an error message if we were the one that created the fatal
        GstFlowReturn value.

        * ext/annodex/gstcmmlutils.c: (gst_cmml_clock_time_from_npt),
        (gst_cmml_clock_time_to_granule), (gst_cmml_track_list_has_clip):
        Parse the seconds field of the npt-sec time format using %llu rather than
        %d and check that the value scaled by GST_SECOND doesn't overflow.
        Use guint64(s) to represent the keyindex and keyoffset fields of a granulepos.
        Lookup a clip's track with clip->track rather than clip->id which
        makes no sense.
        Identify a clip by its track and start time and not its xml id.
        do some more input checking and make sure we don't do undefined shifts.

        * tests/check/elements/cmmldec.c: (setup_cmmldec),
        (teardown_cmmldec), (check_output_buffer_is_equal), (push_data),
        (cmml_tag_message_pop), (check_headers), (push_clip_full),
        (push_clip), (push_empty_clip), (check_output_clip),
        (GST_START_TEST), (cmmldec_suite):
        * tests/check/elements/cmmlenc.c: (setup_cmmlenc),
        (teardown_cmmlenc), (check_output_buffer_is_equal), (push_data),
        (check_headers), (push_clip), (check_clip_times), (check_clip),
        (check_empty_clip), (GST_START_TEST), (cmmlenc_suite):
        Added some more checks.