Last change I see to this call was in 2007, where it was added, not moved.
Does this proposed change fix something, or is it just something that seems more correct to you ?

see change 58ef84846ef9ea695d51930570e83d9f5eedc464, rtpsession: Number of active sources should be updated whenever the status of the source changes to active.

I think the order was changed during forward porting of original patch.

I have not seen an issue with the order but it does seam wrong based on other calls to created in rtpsession.

Thought this bug had the extra info ...

change 58ef84846ef9ea695d51930570e83d9f5eedc464
rtpsession: Number of active sources should be updated whenever the status of the source changes to active

reversed the order, this patch puts it back

I didn't realised that you cared about the order of these signals, committed your patch:

commit 2cf6f84ad63d3ce38fe3134be7e998f5800ad493
Author: Pascal Buhler <pabuhler@cisco.com>
Date:   Fri Jan 13 10:49:43 2012 +0100

    rtpsession: creation should be signaled before validation
    
    https://bugzilla.gnome.org/show_bug.cgi?id=667850

Actually, I wonder if I should do this instead?

if (validated) {
       sess->stats.active_sources++;
       GST_DEBUG ("source: %08x became active, %d active sources", ssrc,
           sess->stats.active_sources);
}

if (created)
      on_new_ssrc (sess, source);
if (validated)
      on_ssrc_validated (sess, source);
if (changed)
      on_ssrc_sdes (sess, source);

Looks fine. Thanks

Now it's not consistent with the RTP reception where on_ssrc_validated is called before the on-new-ssrc.

Well if you ask me then emitting "validated" before "new" makes the "new" signal redundant. This patch was just to revert order back to how the original code was before it got inadvertently changed by a early patch. If you have reasons for reversing the order, ie that "validated" should be before "new" because of behavior xyz than fine reject this patch. Other wise as you pointed out we should  probably change the order of all emissions of new and validate so that they are at least consistent.

(In reply to comment #8)
> Well if you ask me then emitting "validated" before "new" makes the "new"
> signal redundant. This patch was just to revert order back to how the original

I agree, 'new' and 'validated' are a bit redundant

> code was before it got inadvertently changed by a early patch. If you have
> reasons for reversing the order, ie that "validated" should be before "new"
> because of behavior xyz than fine reject this patch. Other wise as you pointed
> out we should  probably change the order of all emissions of new and validate
> so that they are at least consistent.

I inclined to change it so the 'validated' is called before 'new' in all cases because validation is supposed to happen before the SSRC is accepted and considered 'new'.

Well that seams count intuitive to me .. I would say that any time a new ssrc is seen then on_new_ssrc should be called, and only once that ssrc has been validated as good then on_validated_ssrc should be called. But to be fair you did write this right :).

Also although the code in process_rtp calls on_validated first I think in actual fact the first RTP packet of a new SSRC will cause the source to be placed on probation and only after that probation has completed will it be validated. That means the first time on_new_ssrc will be called and the second packet will cause on_validated_to be called.

(In reply to comment #10)
> Well that seams count intuitive to me .. I would say that any time a new ssrc
> is seen then on_new_ssrc should be called, and only once that ssrc has been
> validated as good then on_validated_ssrc should be called. But to be fair you
> did write this right :).

> 
> Also although the code in process_rtp calls on_validated first I think in
> actual fact the first RTP packet of a new SSRC will cause the source to be
> placed on probation and only after that probation has completed will it be
> validated. That means the first time on_new_ssrc will be called and the second
> packet will cause on_validated_to be called.

It is true, I was hoping that on_new_ssrc would only be called for validated new sources but I can see in the code that it isn't..

Because this is the case, I agree that on_new_ssrc goes first and then validated.  I'm wondering however if the on_new_ssrc is done correctly.

AFAIU the spec, you should do minimal processing on SSRCs that you have seen only once because an attacker might simply send random packets and cause massive amounts of on_new_ssrc signals and overflow the session table. Those new unvalidated SSRCs should probably be put into a separate (limited in size) table and the docs for on_new_ssrc should state that this new SSRC might not validate or something.

We should better document what all of these signals are supposed to mean while we're at it.

In the 0.11 branch, the order is new->validated in rtp_session_process_sdes(), but validated->new in rtp_session_process_rtp() ..