On Thu, 8 Jul 2010, Ronald S. Bultje wrote:
> On Thu, Jul 8, 2010 at 4:02 AM, Martin Storsjö <[email protected]> wrote:
> > On Wed, 7 Jul 2010, Josh Allmann wrote:
> >> + if (qdm->timestamp != (uint32_t) -1) {
> >> + pkt->pts = qdm->timestamp;
> >> + qdm->timestamp = -1;
> >> + } else
> >> + pkt->pts = AV_NOPTS_VALUE;
> >
> > This isn't right. (Now I see that something similar slipped through in the
> > svq3 patch, too, but with less impact.)
> >
> > pkt->pts is set (in this case, overwritten) by finalize_packet in
> > rtpdec.c, where it calculates a proper timestamp using RTCP sync and all
> > that.
> >
> > If we want to base that calculation on another RTP timestamp, we need to
> > return the timestamp we want via *timestamp qdm2_parse_packet, so that
> > timestamp is used in the RTP timestamp -> realtime timestamp calculation
> > in finalize_packet.
> >
> > Also, for the follow-up cases, where no data is provided to
> > qdm2_parse_packet but we return some already buffered data, lines 411-415
> > in rtpdec.c, we don't know the RTP timestamp, so it is left at 0 unless
> > the depacketizer overwrites it.
> >
> > The problematic case is if we explicitly want to set pkt->pts to
> > AV_NOPTS_VALUE, since the timestamp pointer/value is an uint32_t, and we
> > can't pass an AV_NOPTS_VALUE there. We could introduce an (uint32_t)-1,
> > which would be interpreted as "don't set pkt->pts".
>
> I think I wrote this to ensure timestamps are increasing (rather than
> staying the same) when a single RTP packet contains multiple QDM2
> frames. Feel free to solve this in any way you think is best, this is
> admittedly a hack.
>
> I haven't looked at the SVQ3 code to see what's up there, but I think
> we shouldn't have to do anything unless you can have multiple SVQ3
> frames in one RTP packet. Timestamp code there can likely be removed
> with no side-effects...
Yes, it shouldn't really be necessary at all there, but setting it (in a
corrected way) may be more correct in some scenarios with dropped packets.
The attached patches allows depacketizers to specify that they want
AV_NOPTS_VALUE by returning RTP_NOTS_VALUE in *timestamp, and fix up the
svq3 depacketizers to handle timestamps correctly.
Josh, when/if we apply this, you should be able to change the code that
does
if (foo)
pkt->pts = qdm->timestamp;
else
pkt->pts = AV_NOPTS_VALUE;
into
if (foo)
*timestamp = qdm->timestamp;
else
*timestamp = RTP_NOTS_VALUE;
// Martin
From 61f741a8e9c24b4e9926326f701cedad5218816f Mon Sep 17 00:00:00 2001
From: Martin Storsjo <[email protected]>
Date: Thu, 8 Jul 2010 22:13:08 +0300
Subject: [PATCH 1/2] Allow depacketizers to specify that pkt->pts should be left as AV_NOPTS_VALUE
---
libavformat/rtpdec.c | 6 ++++--
libavformat/rtpdec.h | 2 ++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index ca06bff..4922ce3 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -375,7 +375,7 @@ rtp_parse_set_dynamic_protocol(RTPDemuxContext *s, PayloadContext *ctx,
*/
static void finalize_packet(RTPDemuxContext *s, AVPacket *pkt, uint32_t timestamp)
{
- if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE) {
+ if (s->last_rtcp_ntp_time != AV_NOPTS_VALUE && timestamp != RTP_NOTS_VALUE) {
int64_t addend;
int delta_timestamp;
@@ -408,7 +408,9 @@ int rtp_parse_packet(RTPDemuxContext *s, AVPacket *pkt,
if (!buf) {
/* return the next packets, if any */
if(s->st && s->parse_packet) {
- timestamp= 0; ///< Should not be used if buf is NULL, but should be set to the timestamp of the packet returned....
+ /* timestamp should be overwritten by parse_packet, if not,
+ * the packet is left with pts == AV_NOPTS_VALUE */
+ timestamp = RTP_NOTS_VALUE;
rv= s->parse_packet(s->ic, s->dynamic_protocol_context,
s->st, pkt, ×tamp, NULL, 0, flags);
finalize_packet(s, pkt, timestamp);
diff --git a/libavformat/rtpdec.h b/libavformat/rtpdec.h
index 86af2b9..aa686da 100644
--- a/libavformat/rtpdec.h
+++ b/libavformat/rtpdec.h
@@ -34,6 +34,8 @@ typedef struct RTPDynamicProtocolHandler_s RTPDynamicProtocolHandler;
#define RTP_MIN_PACKET_LENGTH 12
#define RTP_MAX_PACKET_LENGTH 1500 /* XXX: suppress this define */
+#define RTP_NOTS_VALUE ((uint32_t)-1)
+
typedef struct RTPDemuxContext RTPDemuxContext;
RTPDemuxContext *rtp_parse_open(AVFormatContext *s1, AVStream *st, URLContext *rtpc, int payload_type);
void rtp_parse_set_dynamic_protocol(RTPDemuxContext *s, PayloadContext *ctx,
--
1.7.1
From cc9687f4742ef7cd226c02420373260b53103b37 Mon Sep 17 00:00:00 2001
From: Martin Storsjo <[email protected]>
Date: Thu, 8 Jul 2010 22:14:29 +0300
Subject: [PATCH 2/2] rtpdec_svq3: Return the timestamp in *timestamp instead of pkt->pts
The timestamp of the first RTP packet forming the output AVPacket is
written back in *timestamp, which is used in later calculations in generic
rtpdec code (together with RTCP sync timestamps) to form the final pkt->pts
value.
---
libavformat/rtpdec_svq3.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libavformat/rtpdec_svq3.c b/libavformat/rtpdec_svq3.c
index a285036..af08c88 100644
--- a/libavformat/rtpdec_svq3.c
+++ b/libavformat/rtpdec_svq3.c
@@ -101,7 +101,7 @@ static int svq3_parse_packet (AVFormatContext *s, PayloadContext *sv,
if (end_packet) {
av_init_packet(pkt);
pkt->stream_index = st->index;
- pkt->pts = sv->timestamp;
+ *timestamp = sv->timestamp;
pkt->flags = sv->is_keyframe ? AV_PKT_FLAG_KEY : 0;
pkt->size = url_close_dyn_buf(sv->pktbuf, &pkt->data);
pkt->destruct = av_destruct_packet;
--
1.7.1
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
