membraneframework / membrane_core

The core of the Membrane Framework, advanced multimedia processing framework
https://membrane.stream
Apache License 2.0
1.22k stars 34 forks source link

RTMP plugin resets PTS/DTS values. Why? #790

Closed dmorn closed 1 month ago

dmorn commented 2 months ago

This function is resetting PTS and DTS values of the buffers. Is there any reason behind this decision?

mat-hek commented 2 months ago

AFAIR it's because RTMP uses FLV underneath, and FLV requires timestamps to always start from 0 (see the spec). When I disable the timestamp reset, I see FFmpeg throwing some warnings:

[flv @ 0x1248041a0] Packet with invalid duration -500 in stream 1
[flv @ 0x1248041a0] Failed to update header with correct duration.
[flv @ 0x1248041a0] Failed to update header with correct filesize.

but the tests seem to pass. Not sure how accurate are the checks there though.

dmorn commented 2 months ago

Hi @mat-hek! So our "issue" with that approach is that we're sending the generated RTMP stream to another service that takes care of producing an HLS playlist out of it. That service is designed such that if there is an input loss it still generates empty segments to avoid player stalls. When the connection with the source is re-enstablished, goes on based on the PTS values of the input.

With the plugin designed as-is the whole thing breaks up as the input stream would restart its timing from the beginning, and hence the HLS producer would reject it as it would logically mean that it has to override the whole playlist's contents.

I understand that it may be an unusual setup and you would prefer us to maintain a fork with this behaviour. An alternative could be to add an option to the plugin that makes this functionality optional.

mat-hek commented 2 months ago

Understood, I think we can have such an option ;)

dmorn commented 2 months ago

Great! I'll make a PR soon then