[FFmpeg-soc] [PATCH] qdm2 depacketizer

[Ronald S. Bultje]

Hi,

On Thu, Jul 8, 2010 at 4:02 AM, Martin Storsjö <martin at martin.st> 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...

Ronald