[vlc-devel] [PATCH] Merge VoD module into RTP stream output module

[Pierre Ynard]

> On Fri, 29 Oct 2010 04:01:28 +0200, Pierre Ynard <linkfanel at yahoo.fr>
> wrote:
> > @@ -445,7 +462,43 @@ static int Open( vlc_object_t *p_this )
> >      free (psz);
> >      var_Create (p_this, "dccp-service", VLC_VAR_STRING);
> >  
> > -    if( ( p_sys->psz_destination == NULL ) && !b_rtsp )
> > +    int64_t i_pts_init;
> > +    p_sys->p_vod_media = NULL;
> > +    p_sys->psz_vod_session = NULL;
> > +
> > +    if (! strcmp(p_stream->psz_name, "vod"))
> > +    {
> 
> Why is this not a different Open callback altogether??

Nothing prevents the user from setting up a fixed dst= and/or an
sdp=rtsp:// at the same time. I saw that as a cool feature so I thought
I might as well leave it possible. The only reason I'm using a different
module shortcut now is that since the VLM core passes down variables, I
need a way to make sure that the legacy VoD server module doesn't call
this new code. I see little point in creating a separate submodule.

> > +        /* The VLM stops all instances before deleting a media, so this
> > +         * reference will remain valid during the lifetime of the rtp
> > +         * stream output. */
> > +        p_sys->p_vod_media = var_InheritAddress(p_stream, "vod-media");
> > +
> > +        if (p_sys->p_vod_media != NULL)
> > +        {
> > +            p_sys->psz_vod_session = var_InheritString(p_stream, "vod-session");
> > +            if (p_sys->psz_vod_session == NULL)
> > +            {
> > +                msg_Err(p_stream, "missing VoD session");
> > +                free(p_sys);
> > +                return VLC_EGENERIC;
> > +            }
> > +
> > +            const char *mux;
> > +            if (vod_get_sout_info(p_sys->p_vod_media, p_sys->psz_vod_session,
> > +                                  &mux, &i_pts_init) != VLC_SUCCESS)
> > +            {
> 
> Why do you need to a specific PTS?

The RTSP headers already give the initial RTP timestamps to the client,
before the instance and the rtp sout are started. So I need to use a
common reference, and I need to pass it somewhere.

> > +        if (val == VLC_SUCCESS)
> > +        {
> > +            memcpy(id->ssrc, &ssrc, sizeof(id->ssrc));
> > +            /* This is ugly, but id->i_seq_sent_next needs to be
> > +             * initialized inside vod_init_id() to avoid race
> > +             * conditions. */
> > +            id->i_sequence = id->i_seq_sent_next;
> > +        }
> > +
> > +        /* XXX: It might happen that the VoD media gives us a dynamic
> > +         * payload type that is already in use in p_sys->payload_bitmap,
> > +         * because it was taken by a previous ES that was missing in
> > +         * the VoD media. */
> 
> This is a silly problem. Payload types are scoped to the RTP session
> / the SDP m-line. Since VLC never streams more than one payload
> format per RTP stream, there is no risk of conflict. We might as well
> hard-code value 96 for all dynamic payload types.

We could also implement streaming several payload types in the same RTP
session, that would provide an alternative to TS muxing in order to
stream without needing SDP. I planned on thinking more about that later
and flatten out this code then, that's why I didn't mind that it wasn't
perfect for now.

> > +        /* vod_init_id() may fail either because the ES wasn't found in
> > +         * the VoD media, or because that track wasn't SETUP. In the
> > +         * former case, id->rtp_fmt was left untouched, so we fill it
> > +         * in right after this. */
> > +    }
> > +
> > +    if (id->rtp_fmt.ptname == NULL)
> 
> How can this even happen?! All payload types have specified names, even
> static ones.

Indeed, that's just a test to check if the entire rtp_fmt was set up or
not: if it's NULL, then the structure wasn't initialized at all.

> > +void rtp_get_fmt( vlc_object_t *obj, es_format_t *p_fmt, const char *mux,
> > +                  rtp_format_t *rtp_fmt )
> > +{
> > +    assert( p_fmt != NULL || mux != NULL );
> > +
> > +    rtp_fmt->payload_type = 96; /* dynamic payload type */
> > +    rtp_fmt->ptname = NULL;
> > +    rtp_fmt->cat = mux != NULL ? VIDEO_ES : p_fmt->i_cat;
> > +    if( rtp_fmt->cat == AUDIO_ES )
> > +    {
> > +        rtp_fmt->clock_rate = p_fmt->audio.i_rate;
> > +        rtp_fmt->channels = p_fmt->audio.i_channels;
> > +    }
> > +    else
> > +        rtp_fmt->clock_rate = 90000; /* most common case for video */
> > +    /* Stream bitrate in kbps */
> > +    rtp_fmt->bitrate = p_fmt != NULL ? p_fmt->i_bitrate/1000 : 0;
> > +    rtp_fmt->fmtp = NULL;
> > +
> > +    if( mux != NULL )
> > +    {
> > +        if( strncmp( mux, "ts", 2 ) == 0 )
> > +        {
> > +            rtp_fmt->payload_type = 33;
> > +            rtp_fmt->ptname = "MP2T";
> > +        }
> > +        else
> > +            rtp_fmt->ptname = "MP2P";
> > +        return;
> > +    }
> > +
> [very big switch]
> 
> We really need a new plugin layer for RTp packetization :-( but that's a
> separate problem.

Although they're called twice, once to get RTP parameters, and once to
do the packetization job, that's why there are still two big switches :(
Wouldn't it be weird to load a module when the VoD media is set up, just
to fetch parameters and release it right after?

Thanks for the comments,

-- 
Pierre Ynard
"Une âme dans un corps, c'est comme un dessin sur une feuille de papier."