[Bug 751916] Add GstHarness test framework

GStreamer (GNOME Bugzilla) bugzilla at gnome.org
Mon Jul 6 04:46:21 PDT 2015


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

--- Comment #2 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
Some quick comments/notes/questions/nitpicks:

 - config.h include in installed header file needs to move to .c file

 - does struct _GstHarness need to be public? Should be made private IMHO

 - naming consistency:
   gst_harness_add_element_srcpad/sinkpad() vs. set_src_caps/sink_caps()

 - some functions take ownership of arguments passed, this needs to be
   documented with the appropriate transfer annotation in the doc chunk
   (e.g. gst_harness_set_sink_caps)

 - confusing terminology: (basically caused by leaking implementation details
into the API?):
    - gst_harness_set_src_caps() = input caps, and _set_sink_caps() = output
caps
    - gst_harness_add_src() etc. [wonder if "input" would generally be nicer?]
    - e.g. "Pulls a #GstBuffer from the #GAsyncQueue on the #GstHarness
sinkpad"

 - example in the docs section at the beginning: not even a _play() required?
   what is _play for then?

 - gst_harness_set_pull_mode():
    - docs typo: releasing->release
    - unfortunate naming, since it has nothing to do with GStreamer pull mode

 - gst_harness_try_pull() - why doesn't this also signal on the pull_cond
   if pull_mode_active, just like _pull() does?

 - gst_harness_buffers_received() should do atomic int get;
   also better describe difference to buffers_in_queue in docs
   (received is total so far?); mention that it includes dropped buffers
   - same for _events_received() and _upstream_events_received()

 - gst_harness_set_us_latency() - why 'us', isn't it in nanoseconds?

 - now the idea of a 'src harness' is baked into the API, but internal to
   the harness, I wonder if it would make sense to basically allow setting
   of a src GstHarness instead, and then just call stuff on that?
   (i.e. make the whole thing nestable ultimately) (guess that could always
   be added later if needed)

 - gst_harness_signal() doesn't seem to make any sense?
    - can't pass arguments or retrieve return values
    - should be renamed to emit_signal_by_name()

 - _add_probe() gets destroy_notify for user_data but _connect_signal() not?

 - the comment above gst_harness_stress_push_event_start_full() in the header
   should be in the gtk-doc chunk

 - stress tests: not sure I like the timeout in G_USECs

 - stress tests: multiple stress tests could theoretically be run in parallel
   now, corrct? (I say 'now' because I don't thin that was the case last time
   I looked at the API)

-- 
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