[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