[Bug 709455] [dlnasrc] - New manager type element used for dlna specific HTTP transfers

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=709455
  GStreamer | gst-plugins-bad | 1.x

--- Comment #3 from Olivier Crete (Tester) <olivier.crete at ocrete.ca> 2013-10-05 01:15:32 UTC ---
Thank you for your contribution, it looks very promising.

>From a quick read of the source code, here are some comments:

- In gtk-doc style, at the top of the source file, can you add some
documentation explaining what this is for, what it does, how to use it,
preferably with example pipelines. Can you please include references to the
relevant bits of the DLNA spec? Are these parts public?
- Please run the .c file through gst-indent, although you're already 99% there
- Don't declare variable in the middle of blocks, it should all be C89
- No C++-style // comments, only C-style /* */
- While you're at it, clean up the  comments obvious to people familiar with
GStraemer/GObject
- Function doc should be in gtk-doc style, not doxygen style
- Add G_PARAM_SPEC_STATIC_STRINGS when adding properties
- Please replace the empty dispose() method with a finalize() method and free
everything in the object (mostly the strings?), they're currently being leaked
- Why do you do the head request manually instead of using libsoup? The current
code is not portable to Windows.
- struct_str should probably be a GString instead.. But you should use libsoup
instead of re-implementing HTTP, that would remove a lot of code I think
- What's the point of the cl_name property ?
- It is safe to call g_free (NULL), so if (x) g_free(x); can be replaced with
g_free(x);
- You may want to create a "virtual" protocol like dlnahttp:// and dlnahttps://
to allow it to be auto-plugged. Probably rank it as SECONDARY instead of
PRIMARY-1
- gst_play_marshal_VOID__OBJECT_BOOLEAN is declared but doesn't exist
- Please add a state_change method to GstElement that will fail the NULL->READY
transition if souphttpsrc can't be created
- You need to create the ghostpad in the init function, not when setting the
uri. It is declared as static
- Please don't do the HEAD request in the set URI call, but only on the
READY->PAUSED transition. 
- Don't use g_try_malloc0(sizeof(...))... then not check if the memory was
allocated. Please use g_slice() instead. Generally don't waste time with
g_try_malloc(), GLib assumes all over the place that memory allocations don't
fail (and otherwise calls abort())
- For the npt parser, you may want to look at parse_npt_time() in
gst-plugins-base/gst-libs/gst/rtsp/gstrtsprange.c

Are you planning to also contribute the dtcp-ip support? If you're not, then it
shouldn't be used, elements in the upstream repositories shouldn't rely /use
secret elements. You may want to architect it in a way that allows you to do
dtcp-ip processing in an element that follows the source in your pipeline or
something like that. Or maybe make the dtcpsrc could be a further container
around dlnasrc.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.