[FFmpeg-devel] [PATCH] rtsp - alternate protocol
Ronald S. Bultje
rsbultje
Sat Dec 29 18:41:09 CET 2007
Hi,
On Dec 28, 2007 5:12 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Fri, Dec 28, 2007 at 04:23:45PM -0500, Ronald S. Bultje wrote:
> > @@ -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
I kept the second, in a modified form (see below).
> > @@ -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
I think this is the easiest way. Why else is it a bitmask?
> > @@ -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
Changed as suggested.
> > + } else
>
> else after continue, whats the sense of this?
This is more for indenting consistency, I personally find if/else blocks to
read easier this way, even if the else isn't needed.
> error -> remove bit from protocol_mask
> protocol_mask == 0 -> fail
> else try next
I used this instead now.
> 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
I modified this into explicit ordering as used in _open(), that is the best
way to do it (i.e. even if bit order changes, we're guaranteed to try each
exactly once.
> And please do not send more than a single patch per email and please do
> not
> repost patches, always reply if needed.
What if the second is just a reindent of #1?
New patch (only this one) attached.
Ronald
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: alternate-protocol.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071229/abbbf9bb/attachment.txt>
More information about the ffmpeg-devel
mailing list