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 590643 - refactor HTTP request handling code
refactor HTTP request handling code
Status: RESOLVED FIXED
Product: rygel
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: 0.4
Assigned To: Zeeshan Ali
Depends on:
Blocks:
 
 
Reported: 2009-08-03 15:54 UTC by James Henstridge
Modified: 2009-09-02 23:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-Push-responsibility-for-creating-GStreamer-sources-t.patch (4.31 KB, patch)
2009-08-03 15:54 UTC, James Henstridge
none Details | Review
0002-Factor-out-the-request-handling-code-into-an-HTTPReq.patch (5.58 KB, patch)
2009-08-03 15:55 UTC, James Henstridge
none Details | Review
0002-Factor-out-the-request-handling-code-into-an-HTTPReq.patch (16.68 KB, patch)
2009-08-03 16:05 UTC, James Henstridge
none Details | Review
0001-Push-responsibility-for-creating-GStreamer-sources-t.patch (4.34 KB, patch)
2009-08-25 11:57 UTC, James Henstridge
none Details | Review
0002-Factor-out-the-request-handling-code-into-an-HTTPReq.patch (16.64 KB, patch)
2009-08-25 11:58 UTC, James Henstridge
none Details | Review
0001-Push-responsibility-for-creating-GStreamer-sources-t.patch (4.34 KB, patch)
2009-08-26 11:56 UTC, James Henstridge
committed Details | Review
0002-Factor-out-the-request-handling-code-into-an-HTTPReq.patch (16.69 KB, patch)
2009-08-26 11:57 UTC, James Henstridge
committed Details | Review

Description James Henstridge 2009-08-03 15:54:14 UTC
Attached are some patches to clean up the request handling code a bit.

This gets rid of a bunch of "if (this.transcoder != null)" conditionals in rygel-http-request.vala, and should make it easier to integrate non-GStreamer based transcoders (e.g. a jpeg transcoder or a thumbnailer).

It also fixes a few small bugs:
 * Only set  the tcp-timeout property on the GStreamer source element if
   the element has that property.
 * Fix a bug in the range header generation.  It was not specifying the end
   position if the client sent an open range, even though it knew the length
   of the resource.
 * Only output the Range/TimeSeekRange headers if the corresponding request
   headers were present.
Comment 1 James Henstridge 2009-08-03 15:54:45 UTC
Created attachment 139795 [details] [review]
0001-Push-responsibility-for-creating-GStreamer-sources-t.patch
Comment 2 James Henstridge 2009-08-03 15:55:23 UTC
Created attachment 139796 [details] [review]
0002-Factor-out-the-request-handling-code-into-an-HTTPReq.patch

Part two.
Comment 3 James Henstridge 2009-08-03 16:05:52 UTC
Created attachment 139797 [details] [review]
0002-Factor-out-the-request-handling-code-into-an-HTTPReq.patch

Fixed version of second patch -- forgot to use "git commit -a"
Comment 4 James Henstridge 2009-08-25 11:57:26 UTC
Created attachment 141633 [details] [review]
0001-Push-responsibility-for-creating-GStreamer-sources-t.patch
Comment 5 James Henstridge 2009-08-25 11:58:02 UTC
Created attachment 141634 [details] [review]
0002-Factor-out-the-request-handling-code-into-an-HTTPReq.patch

Updated and rebased patches.
Comment 6 James Henstridge 2009-08-26 11:56:48 UTC
Created attachment 141739 [details] [review]
0001-Push-responsibility-for-creating-GStreamer-sources-t.patch
Comment 7 James Henstridge 2009-08-26 11:57:12 UTC
Created attachment 141740 [details] [review]
0002-Factor-out-the-request-handling-code-into-an-HTTPReq.patch
Comment 8 Zeeshan Ali 2009-08-26 14:29:10 UTC
commit 598a9588351c04d1379f97c9a16aa93e60d0378a
Author: James Henstridge <james@jamesh.id.au>
Date:   Mon Aug 3 21:06:51 2009 +0800

    core: Refactor: request handling into separate interface
    
    Factor out the request handling code into an HTTPRequestHandler interface.
    The interface is implemented by Rygel.Transcoder and an "identity request
    handler" used to serve items as is.

commit c412f9c4a08d54beefe5a34767ef58a6321b5e32
Author: James Henstridge <james@jamesh.id.au>
Date:   Mon Aug 3 18:16:31 2009 +0800

    core: Push creation of gst source to MediaItem
    
    Previously MediaItem.create_stream_source was only called for items that
    did not provide any URIs.  If a URI was available, then HTTPRequest
    would create an element directly from it.  Now HTTPRequest always calls
    create_stream_source and the default implementation knows how to create
    an element from a URI.
Comment 9 Zeeshan Ali 2009-08-26 21:44:03 UTC
This was mostly good stuff and thats why I merged and pushed the changes. However, later while looking at some code I realized that this goes against one of my goals: HTTP-specific stuff be as separate as possible so that later on it's much easier to add more protocols (e.g RTSP) without messing-up the code.

So I propose that we instead:

* Make HTTPRequest an abstract class.
* Merge HTTPRequestHandler into HTTPRequest.
* Move current HTTPRequestHandler implementations into two new subclasses of HTTPRequest.
Comment 10 Zeeshan Ali 2009-09-02 23:30:18 UTC
Issue resolved in rygel git master.