[FFmpeg-devel] [PATCH] rtsp - alternate protocol
Michael Niedermayer
michaelni
Fri Dec 28 23:59:46 CET 2007
On Fri, Dec 28, 2007 at 11:51:37PM +0100, Michael Niedermayer wrote:
> On Fri, Dec 28, 2007 at 11:12:54PM +0100, Luca Abeni wrote:
> > Hi Ronald,
> >
> > Luca Abeni wrote:
> > > Ronald S. Bultje wrote:
> > > [...]
> > >>> If I understand the code correctly, it is explicitly copying in path
> > >>> only the part of the URL contained between "/" and "?".
> > >>>
> > >>> Should it be fixed by removing the "strchr(p, '?')"? Wouldn't this break
> > >>> something else?
> > >>
> > >> That sounds correct, I think you can commit that.
> > > Uhmmm... That was my impression too... But this change would basically
> > > reverse r10775, which claim to fix a regression. So, fixing a bug risks
> > > to re-introduce an old regression... I need to investigate this issue a
> > > little bit more.
> >
> > It seems that the situation is more messy than I expected:
> > 1) url_split() has been rewritten in r10605
> > 2) the rewrite introduced a regression, by changing the behaviour of
> > url_split()
> > 3) r10775 claimed to fix the regression by introducing yet another
> > different behaviour in url_split(). And this is the change that broke
> > option parsing in rtsp.c.
>
> argh ...
> ive just approved r10775 because ronald said it restores original behavior
> i even EXPLICITLY complained that ot would cut the ? part off
> see:
> http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2007-October/036857.html
> and
> http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2007-October/036859.html
>
> you can revert r10775 or any part of it as you see fit!
>
>
> >
> > Some data points:
> > 1) Before r10605, rtsp://localhost:5454/test1-rtsp.mpg?tcp resulted in
> > path="/test1-rtsp.mpg?tcp"
> > 2) After r10605, the same url produced path="/test1-rtsp.mpg?tcp", so no
> > change for this test case...
> > 3) r10775 "fixed" something so that the result is now
> > path="/test1-rtsp.mpg?".
> > 4) So, I tried a different url: rtsp://localhost?tcp. The results are
> > path="?tcp" before r10605, path="" after r10605, and path="" after
> > r10775. Again, that commit does not seem to fix any regression.
> >
> > So, can anyone explain what's the supposed behaviour of url_split()?
> > Which testcase was fixed by r10775?
>
> iam not sure but if i guess i think it prevents ?tcp from ending in the
> hostname
heres a attempt to fix it (completely untested, theres a good chance it
contains trivial mistakes but ill comit it if noone points at a problem
soon *g*)
@@ -2941,7 +2941,7 @@
char *path, int path_size,
const char *url)
{
- const char *p, *ls, *at, *col, *brk, *q;
+ const char *p, *ls, *at, *col, *brk;
if (port_ptr) *port_ptr = -1;
if (proto_size > 0) proto[0] = 0;
@@ -2962,12 +2962,12 @@
}
/* separate path from hostname */
- if ((ls = strchr(p, '/'))) {
- if ((q = strchr(ls, '?')))
- av_strlcpy(path, ls, FFMIN(path_size, q - ls + 1));
- else
+ ls = strchr(p, '/');
+ if(!ls)
+ ls = strchr(p, '?');
+ if(ls)
av_strlcpy(path, ls, path_size);
- } else if (!(ls = strchr(p, '?')))
+ else
ls = &p[strlen(p)]; // XXX
/* the rest is hostname, use that to parse auth/port */
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071228/2c55ee47/attachment.pgp>
More information about the ffmpeg-devel
mailing list