record_interval drops video frames when rotating files

[micolous]

If record_interval is set, when the file is rotated (ie: file 1 is closed, file 2 is opened), the recording will drop all video frames until the next keyframe.

This appears to be a regression introduced by #22.

Steps to reproduce

I have an RTMP endpoint configured to always record incoming video:

record all;
record_path /video;
record_suffix _%Y-%m-%d_%H_%M_%S.flv;
# Hack to trigger this bug more frequently:
record_interval 1m;
# Normally set to 30m.

After setting this configuration, I

1. restarted nginx (to close all files)
2. pushed a h264+aac stream to nginx-rtmp using OBS for 2 minutes
3. stopped the stream from OBS
4. restarted nginx again (to close all files)

I then re-assembled the recorded FLV chunks with ffmpeg:

ffmpeg -f concat -safe 0 -i <(printf "file '$PWD/%s'\n" ./room1_*.flv ) -c copy room1.flv

Expected behaviour

The recorded files should have no stuttering or frame drops, unless there was a problem with the network connection or encoder.

To achieve this, the module should wait until the next video keyframe before triggering a recording rotation.

Actual behaviour

Every time the file changes, all video frames are lost until the next keyframe.

I can see the same issue even when putting the original FLV files in a playlist with VLC.

I can also reproduce the behaviour using StreamYard as an RTMP source, rather than OBS.

Comparison with tcpdump

For comparison, I also recorded the same video stream using tcpdump:

tcpdump -pvn 'tcp port 1935' -i eth0 -w /video/test_stream.pcap

I then used tcpflow and rtmp2flv to extract the RTMP TCP stream from the packet capture, and then reassemble in into an FLV file:

tcpflow -T %T_%A%C%c.rtmp -r /video/test_stream.pcap
python3 rtmp2flv.py *.rtmp

The resulting FLV file was complete, with lost frames. Comparing the two FLV files, I get:

• tcpdump: 87 874 201 bytes
• nginx-rtmp: 87 458 208 bytes

The re-assembled nginx-rtmp file is 415 993 bytes smaller.

Cause

I think the cause of this problem is https://github.com/sergey-dryabzhinsky/nginx-rtmp-module/commit/25146e40f99af59eb3641491a5ec05eff20235fb (#22); it appears to allow rotations to happen on any frame, rather than waiting for brkframe = true, in attempt to support file rotations with manual recording mode.

This behaviour is still present in the current version of the code:

https://github.com/sergey-dryabzhinsky/nginx-rtmp-module/blob/8e344d799483145666fa875344bddf67a324e561/ngx_rtmp_record_module.c#L1106-L1121

The ngx_rtmp_record_node_open call will clear the rctx struct, which then when we get down to writing a video frame, this will fail because rctx->video_key_sent = 0 if the cut was not on a keyframe:

https://github.com/sergey-dryabzhinsky/nginx-rtmp-module/blob/8e344d799483145666fa875344bddf67a324e561/ngx_rtmp_record_module.c#L1195-L1206

Fix

There should be a if (brkframe) check around the record_interval rotation logic again, similar to what the original code had:

https://github.com/sergey-dryabzhinsky/nginx-rtmp-module/blob/afd350e0d8b7820d7d2cfc3fa748217153265ce6/ngx_rtmp_record_module.c#L1044-L1048

But without the exclusion if manual recording is set.