[FFmpeg-devel] [PATCH] A/V Synchronization in the RTP demuxer

[Michael Niedermayer]

On Fri, Jun 20, 2008 at 04:21:23PM +0200, Luca Abeni wrote:
> Hi all,
>
> as some of you might already know, the current implementation
> of the RTP demuxer generates bad timestamps for video != MPEG{1,2}
> and audio != mp{2,3}.
>
> The attached patch should fix this issue (works fine here).
> Comments?
[...]
> @@ -361,31 +365,15 @@
>   */
>  static void finalize_packet(RTPDemuxContext *s, AVPacket *pkt, uint32_t timestamp)
>  {
> -    switch(s->st->codec->codec_id) {
> -        case CODEC_ID_MP2:
> -        case CODEC_ID_MPEG1VIDEO:
> -        case CODEC_ID_MPEG2VIDEO:
>              if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE) {
>                  int64_t addend;
>  
>                  int delta_timestamp;
> -                /* XXX: is it really necessary to unify the timestamp base ? */
>                  /* compute pts from timestamp with received ntp_time */
>                  delta_timestamp = timestamp - s->last_rtcp_timestamp;
> -                /* convert to 90 kHz without overflow */
> -                addend = (s->last_rtcp_ntp_time - s->first_rtcp_ntp_time) >> 14;
> -                addend = (addend * 5625) >> 14;
> +        /* convert to the PTS timebase */
> +        addend = av_rescale_q(s->last_rtcp_ntp_time - s->first_rtcp_ntp_time, AV_TIME_BASE_Q, s->st->time_base);

This looks wrong, last_rtcp_ntp_time is being read from somewhere with
AV_RB64() while AV_TIME_BASE_Q is a lav* specific constant and can hardly
match the timebase of what is read from somewhere.


>                  pkt->pts = addend + delta_timestamp;
> -            }
> -            break;
> -        case CODEC_ID_AAC:
> -        case CODEC_ID_H264:
> -        case CODEC_ID_MPEG4:
> -            pkt->pts = timestamp;

so this was wrong? and s->last_rtcp_ntp_time != AV_NOPTS_VALUE is required
now if one wants timestamps?
no i dunno rtp*.c but i think your explanations are a little terse
i mean what is wrong, why is it wrong and how does your fix improve it?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080620/30fe67c2/attachment.pgp>