[vlc-devel] Patch to specify audio/video payload type for RTP streaming

[Rémi Denis-Courmont]

Le dimanche 13 avril 2014, 21:27:10 Andreas Granig a écrit :
(...)
> However, when doing O/A, there is usually a lot more involved which VLC
> in itself is not going to handle (like the signalling part, and stuff
> like ICE negotation etc), so you'd construct your own SDP in the
> application controlling VLC anyways.

You can't do ICE with VLC anyway, since you cannot multiplex the STUN packets 
on VLC's RTP and RTCP sockets. You would have to proxy the stream; that's 
inefficient at best.

> Having said that, the only thing
> which is necessary to make this work on the streaming level itself is
> setting the ptype id, which is in fact the only thing the patch does.

I don't really agree there. You can have RTCP parameters, or payload type-
specific parameters (e.g. AMR is full of them).

(...)
> > The range 0-127 should be specified, and the default values too, probably
> > 96 and 97.
> 
> Ok. Didn't know about the add_integer_with_range() macro, adapted the
> patch accordingly (my first encounter with the VLC code, so any critics
> are welcome). Setting a default value within the valid range is
> intentionally avoided, as I really want to leave the ptype ids VLC
> chooses in place if the ids are not specified explicitly by the user.

That (existing) code is not terribly useful. It builds on the misconception 
that you cannot have different mappings for the same payload type numbers from 
distinct m=lines. In principles, nothing prevents always using 96 for audio, 
97 for video and 98 for text, or even 96 for everything. Frankly, I'd be in 
favor of removing the current bitmask logic.

As long as BUNDLE is not involved.

> Note that you can specify any ptype id you want, and if you do so, it
> will override also the non-dynamic ids with whatever value you give it
> to give all flexibility.

Your patch is introducing a silent failure case. The payload type may clash 
with RTCP if rtcp-mux is enabled.

> Another option would be to only allow values in the range 96-127 and
> ignore the user-provided setting with a warning if the original payload
> is smaller than 96.

That is tempting to avoid clashes with static payload types and RTCP. But the 
other end is not required to stick to [|96,127|] to my knowledge (even if 
reasonable implementation sticks to it anyway).

> But then again, if you're already going to change
> the default ptype id, you should probably know what you're doing
> anyways, and might have a good reason to change a static payload type id.
> 
> > +    uint16_t  i_ptype_audio;
> > +    uint16_t  i_ptype_video;
> > 
> > uint8_t
> 
> I changed it to int8_t in order to use a default value of -1 in case the
> value is not provided by the user. Is there another option (macro?) to
> check whether or not the parameter has been provided by the user?

Unfortunately, no.

> It
> works by setting a default value of -1 and a range of 0-127, not sure if
> there is a better way though.
> 
> > +        if( p_fmt->i_cat == AUDIO_ES && p_sys->i_ptype_audio > 0 )
> > +            id->rtp_fmt.payload_type = p_sys->i_ptype_audio;
> > +        else if( p_fmt->i_cat == VIDEO_ES && p_sys->i_ptype_video > 0 )
> > +            id->rtp_fmt.payload_type = p_sys->i_ptype_video;
> > 
> > No, zero is a legal value here.
> 
> Right. I changed the default value to -1 and adapted the check accordingly.

-- 
Rémi Denis-Courmont
http://www.remlab.net/