[FFmpeg-devel] [RFC] Synchronize the start times of multiple RTP streams

[Martin Storsjö]

On Sun, 4 Apr 2010, Martin Storsj? wrote:

> On Sun, 4 Apr 2010, Ronald S. Bultje wrote:
> 
> > On Sat, Apr 3, 2010 at 4:34 PM, Martin Storsj? <martin at martin.st> wrote:
> > > The patches in the series are more or less incremental, as in, each give a
> > > small enhancement in behaviour - the more of them you use, the better
> > > things behave. :-)
> > 
> > I'm not sure what the RTP-Info part does, can you describe that?
> 
> The RTSP server may add an RTP-Info header in the reply to PLAY commands, 
> giving hints about the RTP streams, info about the sequence number and 
> start RTP timestamp for each stream. DSS gives this info for static 
> streams from file on disk, but not for relayed live streams.
> 
> The reason is, among other things, to be able to differentiate between RTP 
> packets sent before and after a seek.
> 
> > > +                for (j = 0; j < reply->nb_rtp_info; j++) {
> > > +                    if (!strcmp(reply->rtp_info[j].url,
> > > +                                rt->rtsp_streams[i]->control_url)) {
> > > +                        rtpctx->base_timestamp = reply->rtp_info[i].rtptime;
> > > +                        break;
> > > +                    }
> > > +                }
> > > +                if (j >= reply->nb_rtp_info) {
> > > +                    // No rtp info for this stream?
> > > +                }
> > 
> > You could add an av_log() for those where setting it fails.
> 
> Hmm, perhaps, yes. For live streams in DSS, it adds this header, but 
> doesn't specify any rtptime field, so it wouldn't trigger this case.
> 
> > > +#define RTSP_RTP_INFO_MAX_STREAMS 5
> > [..]
> > > +typedef struct RTPStreamInfo {
> > > +    char url[1024];
> > > +    uint64_t seq;
> > > +    uint64_t rtptime;
> > > +} RTPStreamInfo;
> > 
> > [5] is a little low (I think it's per-stream), why not just directly
> > set it in the AVStream when it's given?
> 
> Yes, it's a bit low, but in the form it's received in ff_rtsp_parse_line, 
> we don't have access to anything else to store it in than in the 
> RTSPMessageHeader struct. But growing that struct to make space for 
> MAX_STREAMS info structs each containing an URL, whent he 
> RTSPMessageHeader is stored on the stack on many places, doesn't feel good 
> either.
> 
> > In general, I definitely think 1-2 are the way to go, and would want
> > ot test 3-4 to see what they do. I'd also like Luca (both)'s inputs,
> > they know the RFCs better than me.
> 
> Attached updated patches, with some more tweaks to the offset handling in 
> #4.

Ping for Luca B on these ones - Ronald was quite ok with at least #1 and 
#2 iirc, what do you whink? Do they work with seeking for you?

// Martin