<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>Am 09.06.2014 um 20:51 schrieb David Fuhrmann <<a href="mailto:david.fuhrmann@gmail.com">david.fuhrmann@gmail.com</a>>:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class="Apple-interchange-newline">Am 09.06.2014 um 20:14 schrieb Rémi Denis-Courmont <<a href="mailto:remi@remlab.net">remi@remlab.net</a>>:</div><br class="Apple-interchange-newline" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Le lundi 9 juin 2014, 19:56:18<span class="Apple-converted-space"> </span><a href="mailto:david.fuhrmann@gmail.com">david.fuhrmann@gmail.com</a><span class="Apple-converted-space"> </span>a écrit :<br><blockquote type="cite">From: David Fuhrmann <<a href="mailto:dfuhrmann@videolan.org">dfuhrmann@videolan.org</a>><br><br>The error case i_pos > tk->i_end implies that<br>p_sys->stream.i_offset > tk->i_end - tk->i_start. This is not allowed<br>as it leads to unsigned underflows in several places. Thus, i_offset<br>is set to a sane value.<br><br>The conversion to int in AStreamRefillStream is necessary to properly<br>detect i_toread < 0, and to avoid the mentioned underflow.<br></blockquote><br>I think you're better off eliminating any case that leads to this, because this<span class="Apple-converted-space"> </span><br>is ostensibly bogus.<br><br><blockquote type="cite"><br>close #11488<br>---<br>src/input/stream.c | 8 +++++---<br>1 file changed, 5 insertions(+), 3 deletions(-)<br><br>diff --git a/src/input/stream.c b/src/input/stream.c<br>index 18e77e2..eafc7a2 100644<br>--- a/src/input/stream.c<br>+++ b/src/input/stream.c<br>@@ -1261,8 +1261,10 @@ static int AStreamSeekStream( stream_t *s, uint64_t<br>i_pos ) if( p_sys->stream.i_used < STREAM_READ_ATONCE / 2 )<br> p_sys->stream.i_used = STREAM_READ_ATONCE / 2;<br><br>- if( AStreamRefillStream( s ) && i_pos >= tk->i_end )<br>+ if( AStreamRefillStream( s ) && i_pos >= tk->i_end ) {<br>+ p_sys->stream.i_offset = tk->i_end - tk->i_start;<br></blockquote><br>This looks fishy. RefillStream should never let i_offset point outside the buffer<span class="Apple-converted-space"> </span><br>to begin with.<br></div></blockquote><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Hi,</div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">In this situation, i_offset is alread pointing outside the buffer before AStreamRefillStream, see a couple of lines before (because i_pos > tk->i_end before the AStreamRefillStream call). Now, according to the the error check, I assumed that this might be temporarily acceptable, and then, AStreamRefillStream is called to read more data so that i_offset is valid again afterwards. The code looks like it was intended to support that.</div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">If this is not intended, then indeed, the wrong i_offset should be corrected already before AStreamRefillStream. But in this case, AFAIU i_pos > tk->i_end should never be true in any way, thus the error check might be redundant.</div></blockquote><div><br></div>Hello again,</div><div><br></div><div>I just tried the alternative proposal, i.e. directly correcting i_offset when it is set to the out of range, with this code before the AStreamRefillStream call:</div><div><br></div><div>if (tk->i_end < tk->i_start + p_sys->stream.i_offset) {<br> p_sys->stream.i_offset = tk->i_end - tk->i_start;<br> // return VLC_EGENERIC;</div><div>}</div><div><br></div><div>This leads to immediate playback problems (or stop of playback). So indeed, it looks like my assumption is correct that it is possible to have a temporarily invalid i_offset value, and AStreamRefillStream will refill the stream so that i_offset gets valid again.</div><div><br></div><div>For the second chunk, you are right that this is not the best way, and I’ll resend a patch avoiding the casts to int.</div><div><br></div><div>With best regards,</div><div>David</div><div><br></div><div><br></div></body></html>