Corrupted dts/pts handling

[Lighty0410]

Hey everyone ! I faced an issue restreaming an mpegts stream. I take an mpegts stream from the udp socket, split elementary streams, and push a packet to another mpegts muxer. And here comes the problem. Sometimes due to the corrupted packet, the dts/pts value also corrupts which breaks the muxer (usually dts). Thus the muxer throws this error - Application provided invalid, non monotonically increasing dts to muxer in stream 2: 15816433050 >= 7599136258 . And i can't proceed with the stream. For example, the dts values have been going like this: 0,1,2,3,4, 10200 (a corrupted packet), 6,7,8. Since the corrupted packet was being pushed to the muxer, it completely negates the next correct values. The problem is i don't have access to a decoder thus i can't calculate pts/dts on my side. I can rebuild the muxer "in-place" or interleave frames, or buffer a chunk of packets and compare their pts/dts but all these methods seem like a crutch. Is there a way to handle this situation properly? Thanks a lot in advance!

[operutka]

Hi @Lighty0410. This is a common problem and, as far as I know, there is no easy solution for it. It's usually being dealt with in the application protocol. The muxer itself has no way of knowing if the input pts/dts sequence is corrupted or if it's intended. If you send your packets over UDP using a custom application protocol, you can add a packet integrity check (e.g. CRC32) and discard all incoming packets that do not pass the integrity check.

You can also use a packet buffer at the receiving end in combination with packet sequence numbers. Then if you receive a sequence of packets ordered by the sequence number, you can verify the timestamp sequence of those packets and discard all packets that would break the monotonically increasing dts sequence. For example, if you receive the following sequence of packets:

seq #:  ...   33   34   35   36   37 ...
dts:    ... 1000 1025 4567 1050 1075 ...

then you can easily determine that packet 35 should be discarded. I guess you probably use packet sequence numbers anyway because UDP does not guarantee that you receive packets in the same order as you sent them.

Anyway, I don't know what application protocol you are using but I strongly suggest sticking to existing protocols like RTP that are designed to deal with packet reordering, detecting corrupted packets etc.

[Lighty0410]

Hi @Lighty0410. This is a common problem and, as far as I know, there is no easy solution for it. It's usually being dealt with in the application protocol. The muxer itself has no way of knowing if the input pts/dts sequence is corrupted or if it's intended. If you send your packets over UDP using a custom application protocol, you can add a packet integrity check (e.g. CRC32) and discard all incoming packets that do not pass the integrity check.

You can also use a packet buffer at the receiving end in combination with packet sequence numbers. Then if you receive a sequence of packets ordered by the sequence number, you can verify the timestamp sequence of those packets and discard all packets that would break the monotonically increasing dts sequence. For example, if you receive the following sequence of packets:

seq #:  ...   33   34   35   36   37 ...
dts:    ... 1000 1025 4567 1050 1075 ...

then you can easily determine that packet 35 should be discarded. I guess you probably use packet sequence numbers anyway because UDP does not guarantee that you receive packets in the same order as you sent them.

Anyway, I don't know what application protocol you are using but I strongly suggest sticking to existing protocols like RTP that are designed to deal with packet reordering, detecting corrupted packets etc.

Thank you a lot for the answer ! I tried using SRT recently and indeed there are no issues with the dts/pts sequence. Here comes another question (slightly unrelated to this topic) regarding the current crate. Right now I'm fixing timestamps due to 26h timestamp rollover in mpegts. And here's a thing that kinda bugs me. Consider this example:

fn substract_timestamp(mut previous: i64, mut current: i64) -> i64 {
    let diff = current - previous;

    if (0x1FFFFFFFF / 2) < -diff {
        return (0x1FFFFFFFF - previous) + current;
    }

    diff
}

fn remux<W: Write, R: Read>(mut demuxer: DemuxerWithStreamInfo<UdpReader>, mut muxer: Muxer<W>) {
    let mut timestamp = 0;
    let mut previous_ts = 0;

    while let Some(mut packet) = demuxer.take().unwrap() {
        let current_ts = packet.dts().timestamp();

        timestamp += substract_timestamp(previous_ts, current_ts);
        previous_ts = current_ts;

        let t_b = packet.time_base();

        packet = packet.with_dts(Timestamp::new(timestamp, t_b));

        muxer.push(packet);
    }
}

Is there a way to avoid the timestamp (Timestamp::new) creation in each iteration of the loop ? Can i set the timestamp directly to the packet's dts ? Overall, maybe a more elegant solution? Thanks in advance !

[operutka]

Is there a way to avoid the timestamp (Timestamp::new) creation in each iteration of the loop ? Can i set the timestamp directly to the packet's dts ? Overall, maybe a more elegant solution?

You don't need to worry about timestamp creation. It's cheap. There is no memory allocation (see https://docs.rs/ac-ffmpeg/0.17.1/src/ac_ffmpeg/time.rs.html#63). It's just a simple struct containing the timestamp itself and its time base. 128 bits in total.

The API is designed this way on purpose to avoid bugs caused by messed up time bases. FFmpeg itself does not use time base aware timestamps and it creates a lot of mess and unnecessary conversions when passing timestamps between muxers/demuxers/encoders/decoders. This library tries to improve it a bit. :) But if you feel that setting packet pts/dts values directly (i.e. without using the Timestamp type) has some added value, feel free to create a pull request. You could solve this easily with two additional methods on Packet and PacketMut:

pub fn with_raw_pts(mut self, pts: i64) -> Self {
    ...
}

pub fn with_raw_dts(mut self, dts: i64) -> Self {
    ...
}

[Lighty0410]

Is there a way to avoid the timestamp (Timestamp::new) creation in each iteration of the loop ? Can i set the timestamp directly to the packet's dts ? Overall, maybe a more elegant solution?

You don't need to worry about timestamp creation. It's cheap. There is no memory allocation (see https://docs.rs/ac-ffmpeg/0.17.1/src/ac_ffmpeg/time.rs.html#63). It's just a simple struct containing the timestamp itself and its time base. 128 bits in total.

The API is designed this way on purpose to avoid bugs caused by messed up time bases. FFmpeg itself does not use time base aware timestamps and it creates a lot of mess and unnecessary conversions when passing timestamps between muxers/demuxers/encoders/decoders. This library tries to improve it a bit. :) But if you feel that setting packet pts/dts values directly (i.e. without using the Timestamp type) has some added value, feel free to create a pull request. You could solve this easily with two additional methods on Packet and PacketMut:

pub fn with_raw_pts(mut self, pts: i64) -> Self {
    ...
}

pub fn with_raw_dts(mut self, dts: i64) -> Self {
    ...
}

Awesome ! Can i create a pull request fixing the rollover issue in mpeg-ts streams too ? Maybe it will be useful for other folks using this library/for ease of use.

[operutka]

Can i create a pull request fixing the rollover issue in mpeg-ts streams too ?

I'm not sure that this library is the right place for dealing with (de)muxer-specific issues. IMHO this should be handled directly by the MPEG-TS demuxer implementation and, therefore, it should be handled by the underlying FFmpeg libs.

[Lighty0410]

Can i create a pull request fixing the rollover issue in mpeg-ts streams too ?

I'm not sure that this library is the right place for dealing with (de)muxer-specific issues. IMHO this should be handled directly by the MPEG-TS demuxer implementation and, therefore, it should be handled by the underlying FFmpeg libs.

Got it. I will not close the issue (I'll link it to the pts PR). Thank you a lot for advices !