Created attachment 78715 [details] [review]
simplification of the neon plugin

The simplification even saves some bytes on libgstneonhttpsrc.a (despite the addition of the caps setting logic): 70214 -> 70026.

Could you do a short summary of the changes and their purpose? It looks to like it's more than just a clean-up.

I am not sure the typefinding addition is a good idea in this case (sources are special IMHO). GstBaseSrc has a "typefind" property already, so the typefinding should be put into GstBaseSrc if it's needed (but I still don't think this is a good idea for push sources).

Typefinding the first buffer and setting caps makes typefinding depend on the configured buffer size (or the length of the first chunk of data we receive) and the accuracy of typefinding decreases considerably with smaller buffer sizes. Why not just let typefind and/or other elements that do typefinding do their job?

For what regards typefinding, I agree with you. That's why I once posted a patch to decodebin to let it handle elements with ANY caps, too, by recursively adding other decodebins. But Wim Taymans wants to keep the typefind logic in the elements: http://bugzilla.gnome.org/show_bug.cgi?id=341524#c18. Therefore this patch to neonhttpsrc.

The simplifications are for example the reduction of the number of parameters of the function set_uri from 6 to 2 and so on. There's no change in the logic.

Created attachment 78804 [details] [review]
simplification of the neon plugin and typefinding

 * try to do typefinding before sending out buffers with no caps
 * simplify the functions _set_uri and _set_proxy
 * remove unused variable ishttps

Thanks, committed (minus the typefinding bit, for the reasons mentioned above, ie. mostly 'sources are special'):

  2007-03-02  Tim-Philipp Müller  <tim at centricular dot net>

        Patch by: Lutz Mueller  <lutz topfrose de>

        * ext/neon/gstneonhttpsrc.c: (gst_neonhttp_src_class_init),
        (gst_neonhttp_src_init), (gst_neonhttp_src_set_property),
        (gst_neonhttp_src_set_uri), (gst_neonhttp_src_set_proxy),
        (gst_neonhttp_src_send_request_and_redirect),
        (gst_neonhttp_src_uri_set_uri):
        * ext/neon/gstneonhttpsrc.h:
          Simplify _set_uri() and _set_proxy() and remove the unused ishttp
          member (#388050).

        * tests/check/elements/neonhttpsrc.c: (GST_START_TEST):
          Fix bogus URI to something that actually exists, otherwise we just
          bypass the test (and also to something that doesn't redirect, since
          neonhttpsrc doesn't seem to handle this very gracefully yet)