ultravideo / uvgRTP

An open-source library for RTP/SRTP media delivery
BSD 2-Clause "Simplified" License
296 stars 84 forks source link

H26x: Aggregation causes NAL units to be sent in different order #211

Closed festlv closed 4 months ago

festlv commented 5 months ago

If I understand correctly, NAL units should be sent in RTP packets in the same order they came from decoder.

The current code in uvgrtp::formats::h26x::push_media_frame causes the transferred NAL units to be reordered in the case when some NAL units at the start of frame can be aggregated, followed by non-aggregatable NAL unit, followed again by one or more aggregatable NAL units. To better illustrate the issue, suppose scl function in the push_media_frame returns the following NAL units:

NAL0, aggregate = true;
NAL1, aggregate = true;
NAL2, aggregate = false;
NAL3, aggregate = true;

the code will aggregate NAL0,1,3 in a single RTP packet, and then send NAL2 in the next packet.

This causes an issue when trying to decode the received packets sequentially.

jrsnen commented 5 months ago

Hi,

are you feeding uvgRTP more than one access unit? The RFC says that only one access unit should be aggregated and since uvgRTP has no way of knowing where one access unit starts and one ends, it would not be possible for us to aggregate it correctly.

BR, Joni

festlv commented 5 months ago

Yes, I'm fairly sure that it's the same access unit (though I can't exclude that I have gaps in my understanding of what exactly is access unit).

I've verified with YUView that the chunks of frame data I'm providing to uvgRTP have the same POC. Also, this chunk of encoded data is what I get when providing a single raw frame to ffmpeg (with zerolatency/rc_lookahead=0 settings, as my application is latency-sensitive).

tampsa commented 4 months ago

Hi,

You can avoid this issue by calling push_frame() with the new RTP_H26X_DO_NOT_AGGR flag introduced in release 3.1.0. It will send the data as single NAL unit packets. Let us know if you run into further problems!

jrsnen commented 4 months ago

Actually, we decided that flag is not enough to fix the issue. Instead, we will end aggregation when a NAL unit requiring fragmentation is encountered. The flag will serve as a temporary fix, though, if aggregation is not needed.

jrsnen commented 4 months ago

Fixed by c91910351fc7b744a247dc403465238ee431c7c2