<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>