[FFmpeg-devel] Another nice RTP bug...

[Michael Niedermayer]

Hi

On Thu, Aug 23, 2007 at 10:42:42AM +0200, Luca Abeni wrote:
> Hi all,
> 
> I was trying to understand why some tools did not like the mpeg1 video 
> stremed by ffmpeg, and I discovered this, in rtp_write_packet():
> 
> [...]
> if (s->first_packet || rtcp_bytes >= 28) {
>          /* compute NTP time */
>          /* XXX: 90 kHz timestamp hardcoded */
>          ntp_time = (pkt->pts << 28) / 5625;
>          rtcp_send_sr(s1, ntp_time);
> [...]
> 
> This looks very buggy for at least two reasons:
> 1) pkt->pts can be AV_NOPTS_VALUE. Well, at least it happens to be 
> sometimes AV_NOPTS_VALUE, if I use "-vcodec copy".
> If "pkt->pts == AV_NOPTS_VALUE" is not valid, then there is a bug 
> somewhere else in libavcodec of ffmpeg.c... Let me know, and I'll search 
> for it.

-fflags genpts should fill in the pts, yes this should be automatic
each AVOutputFormat should have a flag indicating if it needs pts
and if we do -?codec copy and the muxer needs pts then genpts should
be activated
alternatively we could enable genpts always for -?codec copy that would
be fine as well and simpler so iam slightly in favor of that ...


> If "pkt->pts == AV_NOPTS_VALUE" is valid, then rtp_write_packet() has a 
> bug (no surprise here ;-).
> 
> I personally think that the correct thing to do would be "ntp_time = 
> av_gettime()" - RFC 3550 says "NTP timestamp: Indicates the wallclock 
> time (see Section 4) when this report was sent...". What do other people 
> think about it?

there is a problem here pts are presentation timestamps that is the time
at which the frame should be presented to the user so valid pts may be

1 5 2 3 4 9 6 7 8 

for a stream with b frames (the frames get reordered and displayed in a
order different from the order in which they get transmitted ...)

and av_gettime() will give you 1 2 3 4 5 these are invalid for the
example here

its of course possible that the timestamps are really not pts at all in
which case all our disscussions above are meaningless and we should restart
with AVPacket.dts or av_gettime()
(only the rfc could awnser what type of timestamp should be used ...)
and for what the timestamp is used, audio video sync? nothing? the precisse
time at which the frame should be decoded, presented, something else?
this needs to be clarified before we can decide if pts/dts/av_gettime()
is correct

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- 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/20070823/73dd02c6/attachment.pgp>