[icecast-dev] Patches - Was: Stream metadata settings

[Michael Smith]

On Tuesday 13 January 2004 17:15, Melanie wrote:
> Hi,
>
> On 2004.01.13 02:32 Michael Smith wrote:
> > Since we've now (finally!) got 2.0 out the door, I'm now coming back to
> > these
> > patches - hoping to actually get the functionality incorporated.
> >
> > I've got a few questions, though:
> > 	1) Documentation: did you ever manage to write any? The options
> > you added are
> > fairly straightforward for the most part, so this shouldn't be too hard.
>
> I put that off until I get some positive confirmation that the patches
> _will_ get incorporated. Naturally, I didn't want to waste my time
> documenting patches that would never make it into Icecast in the first
> place.
> However, I'm not refusing to write it, expect some docs when I hear the
> patches are definitely going in.

That's fair enough. The multilevel fallbacks patch (perhaps with 
modifications, but certainly with the functionality intact) is definately 
going in (sometime this week, depending on when I have time). I haven't 
looked at the others in detail.

>
> > 	2) The 'no-mount' option: why? Does this have any effect at all
> > that isn't
> > handled by setting max-listeners to zero?
>
> Fallback will add the clients from the upper level source to the fallback's
> pending queue. IIRC, that's where max-listeners gets checked. Setting
> max-listeners to zero would disallow any listeners at all. No-mount will
> only prevent mounting the source directly, by name, it will not prevent
> listening to it as a fallback. That was the idea.

The no-mount option gets checked (in your patch) in the same codepath as the 
(per-mount and global, though global doesn't matter here) max-listeners. 

Though that indicates a bug somewhere - these (max-listeners and your new 
no-mount) get checked when adding to the pending queue, but the current 
number of listeners doesn't get incremented until the source pulls the client 
off the pending queue. 

There's no check for either of these parameters when the source pulls the 
client(s) off the pending queue - so fallbacks are unaffected.

So: either no-mount goes (because it's useless), or we change things so that 
no-mount prevents direct mounts (only), and max-listeners is absolute (and is 
enforced when the source pulls clients off the pending queue).

The latter makes more sense, I guess, but is a significant change in 
behaviour. Does anyone else have any opinions on this?

<p>>
> > 	3) In the multilevel fallbacks patch, you've added a comment
> > saying
> > "Likely safe to do since there can only be one thread, ever", along with
> > some
> > static local variables, in the source_find_mount() function. I'm not sure
> > why
> > you said this, it's clearly incorrect: the source_* functions are called
> > from
> > many different threads, and this is clearly not safe. This is easily
> > fixed,
> > though, by passing the neccesary variables recursively, and splitting the
> > function into two.
>
> AFAIK, checking the function references I found that a certain lock must be
> hald to call it, off the top of my head I don't recall which. Since the
> function must be called under lock, there should only be one thread az a
> time executing it.
> I wasn't 100% sure about this, that's why I added the comment, so someone
> else would have a look.
>

Almost right. A lock has to be held (the global.source_tree lock), but that's 
an rwlock, which allows multiple simultaneous readers. I'll redo the code 
there.

Mike

--- >8 ----
List archives:  http://www.xiph.org/archives/
icecast project homepage: http://www.icecast.org/
To unsubscribe from this list, send a message to 'icecast-dev-request at xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is needed.
Unsubscribe messages sent to the list will be ignored/filtered.