[FFmpeg-soc] [PATCH] rtsp tunneling

[Josh Allmann]

On 5 June 2010 12:01, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> Hi,
>
> On Sat, Jun 5, 2010 at 4:28 AM, Josh Allmann <joshua.allmann at gmail.com> wrote:
>> Fixed that, and other minor errors (s/handler/handle, spelling fixes
>> in the proper commit, etc). Soc repo has been updated as well.
> [..]
>> +    if (!s->init) {
>> +        int ret = http_open_cnx(h);
>> +        if (ret != 0) {
>> +            av_free (s);
>
> The av_free(s) should be removed in read/write, otherwise subsequent
> calls to read/write (or close) will/might crash.

Fixed.

>
>> +/**
>> + * Sets custom HTTP headers. Note this overwrites all default options.
>> + *
>> + * The following headers will always be set, no matter what:
>> + *  -HTTP verb, path, HTTP version
>> + *  -User-Agent
>> + *  -Transfer-Encoding (if applicable)
>> + *  - Authorization (if applicable)
>> + *
>> + * The following headers are set by default, and will be overwritten if
>> + * custom headers are set. Be sure to re-specify them if needed.
>> + *  -Accept
>> + *  -Range
>> + *  -Host
>> + *  -Connection
>> + *
>> + * @param h URL context for this HTTP connection
>> + * @param is_chunked 0 to disable chunking, nonzero otherwise.
>> + */
>> +void ff_http_set_headers(URLContext *h, const char *headers);
>
> You shouldn't omit them if missing from the "custom" header. The
> expected behaviour if custom header is "Accept: bla\r\n" is that the
> default Host is kept (same for the others). Also, please document that
> the caller is required to maintain trailing "\r\n" in the custom
> header. Assert this in the implementation. Also document that "\0"
> resets the custom header.
>

Fixed on all counts.
I was wrong earlier; adding in the default headers don't break stuff,
so we don't need to omit anything.

>> @@ -391,7 +398,7 @@ static int http_read(URLContext *h, uint8_t *buf, int size)
>>  /* used only when posting data */
>>  static int http_write(URLContext *h, const uint8_t *buf, int size)
>>  {
>> -    char temp[11];  /* 32-bit hex + CRLF + nul */
>> +    char temp[11] = "";  /* 32-bit hex + CRLF + nul */
>>      int ret;
>>      char crlf[] = "\r\n";
>>      HTTPContext *s = h->priv_data;
>> @@ -413,11 +420,12 @@ static int http_write(URLContext *h, const uint8_t *buf, int size)
>>       * signal EOF */
>>      if (size > 0) {
>>          /* upload data using chunked encoding */
>> +        if(s->is_chunked)
>>          snprintf(temp, sizeof(temp), "%x\r\n", size);
>>
>>          if ((ret = url_write(s->hd, temp, strlen(temp))) < 0 ||
>
> is_chunked && (ret = url_write... More generally, feel free to unroll this into:
> if (is_chunked){
> snprintf();
> url_write();
> }
> url_write();
> if (is_chunked)
> url_write();
>

Fixed. The code is not as tight now, but a little less gnarly.

> How does the caller or callee define the length of the data if it's
> not chunked? We need some defined way of getting that, otherwise this
> will break depending on the receiving server implementation.
>
> #4 is fine, if Luca is OK (or Martin), please apply.
>
>> +    /** RTSP transport mode, such as plain or tunneled. */
>> +    int mode;
>
> enum RTSPMode mode. RTSPMode is also a little generic, but I don't
> really have suggestions for a better name. Luca?
> RTSPProtocolConnectionMode?

Changed to RTSPControlTransport

>
> I think #6 is OK, again Luca/Martin for second review and then it can
> be applied.
>
> Ronald
> _______________________________________________
> FFmpeg-soc mailing list
> FFmpeg-soc at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Modified-behavior-of-http_open-to-implicitly-delay-c.patch
Type: text/x-patch
Size: 2602 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100605/8bff0e2f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Added-in-capability-to-write-custom-HTTP-headers.patch
Type: text/x-patch
Size: 5200 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100605/8bff0e2f/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Added-support-for-disabling-chunked-transfer-encodin.patch
Type: text/x-patch
Size: 3233 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100605/8bff0e2f/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-Added-in-RTSP-tunneling-over-HTTP.patch
Type: text/x-patch
Size: 6857 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100605/8bff0e2f/attachment-0003.bin>