<div dir="ltr"><br><div class="gmail_extra"><br></div><div class="gmail_quote">On Wed, Jul 5, 2017 at 6:28 PM, Zhao Zhili <span dir="ltr"><<a href="mailto:wantlamy@gmail.com" target="_blank">wantlamy@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid"><div dir="ltr"><p>---<br> modules/demux/adaptive/http/H<wbr>TTPConnection.cpp | 66 +++++++++++++++++---------<br> modules/demux/adaptive/http/H<wbr>TTPConnection.hpp | 16 +++++--<br> 2 files changed, 55 insertions(+), 27 deletions(-)</p><p>diff --git a/modules/demux/adaptive/http/<wbr>HTTPConnection.cpp b/modules/demux/adaptive/http/<wbr>HTTPConnection.cpp<br>index a250d87..0b2f7f8 100644<br>--- a/modules/demux/adaptive/http/<wbr>HTTPConnection.cpp<br>+++ b/modules/demux/adaptive/http/<wbr>HTTPConnection.cpp<br>@@ -65,7 +65,6 @@ HTTPConnection::HTTPConnection<wbr>(vlc_object_t *p_object_, Socket *socket_, bool pe<br>     socket = socket_;<br>     psz_useragent = var_InheritString(p_object_, "http-user-agent");<br>     queryOk = false;<br>-    retries = 0;<br>     connectionClose = !persistent;<br>     chunked = false;<br>     chunked_eof = false;<br>@@ -108,27 +107,24 @@ void HTTPConnection::disconnect()<br>     socket->disconnect();<br> }<br> <br>-int HTTPConnection::request(const std::string &path, const BytesRange &range)<br>+HTTPConnection::HTTPErrorType HTTPConnection::doRequest(cons<wbr>t BytesRange &range)<br> {<br>     queryOk = false;<br>     chunked = false;<br>     chunked_eof = false;<br>     chunkLength = 0;<br> <br>-    /* Set new path for this query */<br>-    params.setPath(path);<br>-<br>     msg_Dbg(p_object, "Retrieving %s @%zu", params.getUrl().c_str(),<br>                        range.isValid() ? range.getStartByte() : 0);<br> <br>     if(!connected() && ( params.getHostname().empty() || !connect() ))<br>-        return VLC_EGENERIC;<br>+        return HTTPGenericError;<br> <br>     bytesRange = range;<br>     if(range.isValid() && range.getEndByte() > 0)<br>         contentLength = range.getEndByte() - range.getStartByte() + 1;<br> <br>-    std::string header = buildRequestHeader(path);<br>+    std::string header = buildRequestHeader();<br>     if(connectionClose)<br>         header.append("Connection: close\r\n");<br>     header.append("\r\n");<br>@@ -140,17 +136,20 @@ int HTTPConnection::request(const std::string &path, const BytesRange &range)<br>         {<br>             /* server closed connection pipeline after last req. need new */<br>             connectionClose = true;<br>-            return request(path, range);<br>+            return doRequest(range);<br>+        }<br>+        else<br>+        {<br>+            return HTTPGenericError;<br>         }<br>-        return VLC_EGENERIC;<br>     }<br> <br>-    int i_ret = parseReply();<br>-    if(i_ret == VLC_SUCCESS)<br>+    HTTPConnection::HTTPErrorType i_ret = parseReply();<br>+    if(i_ret == HTTPSuccess)<br>     {<br>         queryOk = true;<br>     }<br>-    else if(i_ret == VLC_ETIMEOUT) /* redir */<br>+    else if(i_ret == HTTPRedirect) /* redir */<br>     {<br>         socket->disconnect();<br>         if(locationparams.getScheme().<wbr>empty())<br>@@ -159,19 +158,40 @@ int HTTPConnection::request(const std::string &path, const BytesRange &range)<br>             params = locationparams;<br>         locationparams = ConnectionParams();<br>     }<br>-    else if(i_ret == VLC_EGENERIC)<br>+    else if(i_ret == HTTPGenericError)<br>     {<br>         socket->disconnect();<br>         if(!connectionClose)<br>         {<br>             connectionClose = true;<br>-            return request(path, range);<br>+            return doRequest(range);<br>         }<br>     }<br> <br>     return i_ret;<br> }<br> <br>+int HTTPConnection::request(const std::string &path, const BytesRange &range)<br>+{<br>+    /* Set new path for this query */<br>+    params.setPath(path);<br>+<br>+    for(int retries = 0; retries < maxRedirectCount; retries++)<br>+    {<br>+        HTTPConnection::HTTPErrorType ret = doRequest(range);<br>+        if (ret == HTTPSuccess)<br>+            return VLC_SUCCESS;<br>+        else if (ret == HTTPGenericError)<br>+            return VLC_EGENERIC;<br>+        else if (ret == HTTPRedirect)<br>+            continue;<br>+        else<br>+            break;<br>+    }<br>+<br>+    return VLC_EGENERIC;<br>+}<br>+<br> ssize_t HTTPConnection::read(void *p_buffer, size_t len)<br> {<br>     if( !connected() ||<br>@@ -215,17 +235,17 @@ bool HTTPConnection::send(const void *buf, size_t size)<br>     return socket->send(p_object, buf, size);<br> }<br> <br>-int HTTPConnection::parseReply()<br>+HTTPConnection::HTTPErrorType HTTPConnection::parseReply()<br> {<br>     std::string line = readLine();<br> <br>     if(line.empty())<br>-        return VLC_EGENERIC;<br>+        return HTTPGenericError;<br> <br>     if (line.compare(0, 9, "HTTP/1.1 ")!=0)<br>     {<br>         if(line.compare(0, 9, "HTTP/1.0 ")!=0)<br>-            return VLC_ENOOBJ;<br>+            return HTTPGenericError;<br>         else<br>             connectionClose = true;<br>     }<br>@@ -254,15 +274,15 @@ int HTTPConnection::parseReply()<br>        !locationparams.getUrl().empty<wbr>())<br>     {<br>         msg_Info(p_object, "%d redirection to %s", replycode, locationparams.getUrl().c_str(<wbr>));<br>-        return VLC_ETIMEOUT;<br>+        return HTTPRedirect;<br>     }<br>     else if (replycode != 200 && replycode != 206)<br>     {<br>-        msg_Err(p_object, "Failed reading %s: %s", params.getUrl().c_str(), line.c_str());<br>-        return VLC_ENOOBJ;<br>+        msg_Err(p_object, "%d Failed reading %s: %s", replycode, params.getUrl().c_str(), line.c_str());<br>+        return HTTPGenericError;<br>     }<br> <br>-    return VLC_SUCCESS;<br>+    return HTTPSuccess;<br> }<br> <br> ssize_t HTTPConnection::readChunk(void *p_buffer, size_t len)<br>@@ -360,10 +380,10 @@ void HTTPConnection::onHeader(const std::string &key,<br>     }<br> }<br> <br>-std::string HTTPConnection::buildRequestHe<wbr>ader(const std::string &path) const<br>+std::string HTTPConnection::buildRequestHe<wbr>ader() const<br> {<br>     std::stringstream req;<br>-    req << "GET " << path << " HTTP/1.1\r\n" <<<br>+    req << "GET " << params.getPath() << " HTTP/1.1\r\n" <<<br>            "Host: " << params.getHostname() << "\r\n" <<<br>            "Cache-Control: no-cache" << "\r\n" <<<br>            "User-Agent: " << std::string(psz_useragent) << "\r\n";<br>diff --git a/modules/demux/adaptive/http/<wbr>HTTPConnection.hpp b/modules/demux/adaptive/http/<wbr>HTTPConnection.hpp<br>index 85ca53b..707e604 100644<br>--- a/modules/demux/adaptive/http/<wbr>HTTPConnection.hpp<br>+++ b/modules/demux/adaptive/http/<wbr>HTTPConnection.hpp<br>@@ -73,6 +73,15 @@ namespace adaptive<br>                 void setUsed( bool );<br> <br>             protected:<br>+                enum HTTPErrorType<br>+                {<br>+                    HTTPSuccess = 0,<br>+                    HTTPGenericError,<br>+                    HTTPRedirect,<br>+                };<br>+<br>+                virtual HTTPConnection::HTTPErrorType doRequest(const BytesRange &range);<br>+<br>                 virtual bool    connected   () const;<br>                 virtual bool    connect     ();<br>                 virtual void    disconnect  ();<br>@@ -82,10 +91,10 @@ namespace adaptive<br>                 virtual void    onHeader    (const std::string &line,<br>                              <wbr>                const std::string &value);<br>                 virtual std::string extraRequestHeaders() const;<br>-                virtual std::string buildRequestHeader(const std::string &path) const;<br>+                virtual std::string buildRequestHeader() const;<br> <br>                 ssize_t         readChunk   (void *p_buffer, size_t len);<br>-                int parseReply();<br>+                HTTPConnection::HTTPErrorType parseReply();<br>                 std::string readLine();<br>                 char * psz_useragent;<br> <br>@@ -95,8 +104,7 @@ namespace adaptive<br>                 bool                chunked_eof;<br>                 size_t              chunkLength;<br>                 bool                queryOk;<br>-                int                 retries;<br>-                static const int    retryCount = 5;<br>+                static const int    maxRedirectCount = 5;<br> <br>             private:<br>                 Socket *socket;<span class="m_9026715744718008573gmail-HOEnZb"><font color="#888888"><br>-- <br>2.7.4<span><span><br></span></span></font></span></p></div>
</blockquote></div><p>Ping for review.</p><div>HTTPChunkSource::prepare() didn't handle redirect to a different path since it always pass</div><div>the old path to connection->request(params.<wbr>getPath(), bytesRange).</div><div><br></div><div>There are two choices:</div><div>1. Add an interface to extract the new params from AbstractConnection</div><div>2. Handle redirect in HTTPConnection::request().</div><div><br></div><div>Since StreamUrlConnection::request() already handle redirect by access_New(), I prefer the</div><div>second choice.</div><div><br></div><div>Any opinion is welcome.<span><span><span><span><span><span><span><span><span><span><span><span><br></span></span></span></span></span></span></span></span></span></span></span></span></div></div>