[FFmpeg-devel] [PATCH] rtsp - alternate protocol

[Michael Niedermayer]

On Fri, Dec 28, 2007 at 04:23:45PM -0500, Ronald S. Bultje wrote:
> Hi,
> 
> On Dec 28, 2007 3:53 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > On Fri, Dec 28, 2007 at 02:57:08PM -0500, Ronald S. Bultje wrote:
> > > On Dec 28, 2007 2:42 PM, Aurelien Jacobs <aurel at gnuage.org> wrote:
> > > > Either an #ifdef VERSION or a version bump. I personaly don't care.
> > >
> > > No need yet for a version bump, this stuff will go away by itself... New
> > > version of patch#3 attached.
> >
> > patch ok
> 
> 
> Here's a re-post of all three then, just to make it easier to apply.
> 
> Ronald

> Index: ffmpeg/libavformat/rtsp.c
> ===================================================================
> --- ffmpeg.orig/libavformat/rtsp.c	2007-12-28 13:30:50.000000000 -0500
> +++ ffmpeg/libavformat/rtsp.c	2007-12-28 13:33:47.000000000 -0500
> @@ -842,6 +842,25 @@
>      av_free(rt->rtsp_streams);
>  }
>  
> +static inline int
> +num_bits_set(int mask)
> +{
> +    int i, n = 0;;
> +    for (i = 0; i < sizeof(int) * 8; i++)
> +        if (mask & (1 << i))
> +            n++;
> +    return n;
> +}
> +static inline int
> +unset_lowest_bit(int mask)
> +{
> +    int i;
> +    for (i = 0; i < sizeof(int) * 8; i++)
> +        if (mask & (1 << i))
> +            return mask &~ (1 << i);
> +    return 0;
> +}

forget this mess
it could be done with one line x&(x-1) but besides i dont see why
any of this would be needed


> +
>  static int rtsp_read_header(AVFormatContext *s,
>                              AVFormatParameters *ap)
>  {
> @@ -883,7 +902,8 @@
>      }
>  
>      if (!protocol_mask)
> -        protocol_mask = rtsp_default_protocols;
> +        for (i = 0; i < RTSP_PROTOCOL_RTP_LAST; i++)
> +            protocol_mask |= (1 << i);

this is (1<<RTSP_PROTOCOL_RTP_LAST) - 1  but i really dont like it


>  
>      /* open the tcp connexion */
>      snprintf(tcpname, sizeof(tcpname), "tcp://%s:%d", host, port);
> @@ -978,6 +998,11 @@
>                   "Transport: %s\r\n",
>                   rtsp_st->control_url, transport);
>          rtsp_send_cmd(s, cmd, reply, NULL);
> +        if (reply->status_code == 461 /* Unsupported protocol */ && i == 0 &&
> +            num_bits_set(protocol_mask) >= 2) {
> +            protocol_mask = unset_lowest_bit(protocol_mask);

> +            j = RTSP_RTP_PORT_MIN; i--; continue;

NO, try to write clean code, please!
or it will take till 2009 to get this into commitable shape

> +        } else

else after continue, whats the sense of this?


error -> remove bit from protocol_mask
protocol_mask == 0 -> fail
else try next

Also there is NO reason why the protocols would be choosen in the same
order as the bits, unset_lowest_bit() is a fragile hack
when someone changes the order it breaks silently

[...]

[rtsp_default_protocols]

Still ok, though its badly incomplete but its ok as first step, it does
no harm as is ...

And please do not send more than a single patch per email and please do not
repost patches, always reply if needed.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- 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/42e9417a/attachment.pgp>