Created attachment 100536 [details] [review]
Patch.

This also implements seek, but only in READY so far, which isn't very useful. That's another TODO which requires more effort...

You could use g_value_dup_string() instead of strdup(g_value_get_string()).

Also for seeking you could keep track of your current position and if current_position!=wanted_position in create() you could destroy your connection and create a new one with the correct range.

Thanks for the g_value_dup_string() tip; it was a copy and paste from neonhttpsrc.

If I want to know the required offset, I'll have to drop PushSrc as a base class because it masks that from the arguments to create(). I guess I can fall back to the default implementation of do_seek() in BaseSrc then?

Created attachment 100692 [details] [review]
Patch to CVS HEAD supporting seek

This implements seeking. However, there is no practical use for this. None of the other HTTP sources support seeking (correctly).
I tried mp3 audio, MPEG-1 video (fails to convert time seek to byte seek), QuickTime (does not even try to seek) and motion JPEG AVI (avidemux crashes).
As this is not properly tested for leaks, I'm just attaching this for test purposes. I would not recommend actual use.

Created attachment 100694 [details] [review]
Patch to CVS HEAD without seek

This:
* fixes the suggestion about strdup,
* removes references to seeking abilities,
* does not try to unpause I/O in the "queued" state,
* reorganises a bunch of things,
* uses G_GUINT64_FORMAT instead of hard-coding %llu.

        Patch by: Wouter Cloetens <wouter at mind dot be>

        * ext/soup/gstsouphttpsrc.c: (_do_init),
        (gst_souphttp_src_class_init), (gst_souphttp_src_init),
        (gst_souphttp_src_dispose), (gst_souphttp_src_set_property),
        (gst_souphttp_src_get_property), (unicodify),
        (gst_souphttp_src_unicodify), (gst_souphttp_src_create),
        (gst_souphttp_src_start), (gst_souphttp_src_stop),
        (gst_souphttp_src_unlock), (gst_souphttp_src_unlock_stop),
        (gst_souphttp_src_get_size), (gst_souphttp_src_is_seekable),
        (soup_got_headers), (soup_got_body), (soup_finished),
        (soup_got_chunk), (soup_response), (soup_parse_status),
        (gst_souphttp_src_uri_get_type),
        (gst_souphttp_src_uri_get_protocols),
        (gst_souphttp_src_uri_get_uri), (gst_souphttp_src_uri_set_uri),
        (gst_souphttp_src_uri_handler_init):
        * ext/soup/gstsouphttpsrc.h:
        Do not try to unpause I/O in the "queued" state.
        Reorganise a bunch of things and cleanups.
        Uses G_GUINT64_FORMAT instead of hard-coding %llu.
        See #502335.

The seeking thing is still useful but likely needs some more work, I'm not closing this yet.

You probably wanted to put the change list in comment 0 in the ChangeLog instead of the list of incremental improvements to that in comment 6.
I think I'm going to need more input from you guys to understand the expected seek behaviour...

Created attachment 100772 [details] [review]
use gst_tag_freeform_string_to_utf8()

Ideally you should also use gst_tag_freeform_string_to_utf8() here instead of the home-baked version that doesn't take into account any environment variables (I've just fixed up gnomevfssrc to do the same, but can't test with souphttpsrc because my soup version is too old).  Would appreciate it if someone could test the patch.

Created attachment 100822 [details] [review]
use gst_tag_freeform_string_to_utf8() and post tags on bus

Post Icecast/Shoutcast tags on bus.

(obsoletes attachment 100772 [details] [review])

Created attachment 100823 [details] [review]
use gst_tag_freeform_string_to_utf8() and post tags on bus

Fix snarf & barf error in that last patch.

Should this bug still be set to 'new' if the patch has already been committed?

Without some guidance on what to do with seeking functionality, I'm stuck there.

Created attachment 103100 [details] [review]
Seek support for souphttpsrc.

Support seek.

Created attachment 103101 [details] [review]
Seek support for souphttpsrc.

Seek support.


The seek patch is updated to apply against CVS HEAD. It has been tested and actually works, in contrast to the original patch. I accidentally left 2 lines of debug cruft in the previous patch.

Thanks, committed.

Created attachment 103167 [details] [review]
Set segment size when it is known.

Fix Totem failing to query the duration of the stream.

Thanks, committed too :)

Created attachment 103228 [details] [review]
Fix seek bug.

Report the size of the stream as the total size instead of the remaining Content-Length, which is wrong after a seek.

Thanks, and again committed :)

This bug can be closed now, I suppose? Everything has been committed.
Further development in bug 510708.