After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 796773 - splitmuxsink creates files that are too large
splitmuxsink creates files that are too large
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other Linux
: Normal normal
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-07-10 01:33 UTC by Pedro Corte-Real
Modified: 2018-07-17 00:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GST_DEBUG=splitmuxsink:6 output (301.78 KB, text/plain)
2018-07-10 01:34 UTC, Pedro Corte-Real
  Details
splitmux: Improve handling of repeated timestamps (2.77 KB, patch)
2018-07-16 14:05 UTC, Jan Schmidt
none Details | Review

Description Pedro Corte-Real 2018-07-10 01:33:44 UTC
I am reading and splitting a RTSP/H264 stream from an IP camera into several ~2s MP4 files with a pipeline that is basically:

gst-launch-1.0 uridecodebin location="rtsp://..." ! h264parse ! splitmuxsink location="test-%d.mkv" max-size-time="100000000" max-size-bytes="1000000"

The only thing I do besides this is gst_base_parse_set_pts_interpolation(h264parse, true) so that the PTS on I-frames is interpolated and the muxing works.

This almost works but results in two types of strange files:

$ du -sh *.mp4
4.0K	test-00000.mp4
1.2M	test-00001.mp4
1.2M	test-00002.mp4
1.2M	test-00003.mp4
1.2M	test-00004.mp4
1.2M	test-00005.mp4
1.2M	test-00006.mp4
1.3M	test-00007.mp4
1.3M	test-00008.mp4
1.3M	test-00009.mp4
1.2M	test-00010.mp4
1.2M	test-00011.mp4
1.2M	test-00012.mp4
1.3M	test-00013.mp4
1.3M	test-00014.mp4
1.2M	test-00015.mp4
1.2M	test-00016.mp4
1.2M	test-00017.mp4
1.2M	test-00018.mp4
1.3M	test-00019.mp4
14M	test-00020.mp4
1.2M	test-00021.mp4
1.2M	test-00022.mp4
1.2M	test-00023.mp4
1.3M	test-00024.mp4
(...)

The first file is a broken file with no content, probably from the first frame from the camera not being an I-frame. And then every so often there are some larger files that should have been cut into smaller ones but werent.

I've attached the splitmuxsink debug output. I also have a gdppay rtspsrc output that allows me to reproduce the issue from a filesrc to not depend on the camera. Since this is a surveillance camera I'll have to film something else to be able to share the file.
Comment 1 Pedro Corte-Real 2018-07-10 01:34:59 UTC
Created attachment 372986 [details]
GST_DEBUG=splitmuxsink:6 output
Comment 2 Pedro Corte-Real 2018-07-15 14:05:22 UTC
A sample file that shows this problem can be downloaded here:

http://scratch.corujas.net/sample.gdp

It can be tested with this pipeline:

gst-launch-1.0 filesrc location=sample.gdp ! gdpdepay ! rtph264depay ! h264parse ! splitmuxsink muxer="matroskamux" location="test-%05d.mp4"

That results in a single file because all the I-frames are missing PTS. Enabling PTS interpolation in h264parse (can't be done in gst-launch) is half a fix. The end result is this:

$ du -sh *.mp4
4.0K	test-00000.mp4
564K	test-00001.mp4
564K	test-00002.mp4
560K	test-00003.mp4
564K	test-00004.mp4
564K	test-00005.mp4
564K	test-00006.mp4
564K	test-00007.mp4
564K	test-00008.mp4
564K	test-00009.mp4
564K	test-00010.mp4
564K	test-00011.mp4
564K	test-00012.mp4
564K	test-00013.mp4
564K	test-00014.mp4
5.6M	test-00015.mp4
564K	test-00016.mp4
568K	test-00017.mp4
568K	test-00018.mp4
568K	test-00019.mp4
568K	test-00020.mp4
568K	test-00021.mp4
568K	test-00022.mp4
568K	test-00023.mp4
568K	test-00024.mp4
572K	test-00025.mp4
572K	test-00026.mp4
2.5M	test-00027.mp4

The first file is broken with no content and then some times there's a large file. That happens also because the PTS is wrong and thus "splitmux->max_in_running_time >= ctx->in_running_time" is true for some I-frames.

I have a gstreamer git build working now so if anyone has a pointer on how to fix the missing PTS on these streams properly I'll have a look.
Comment 3 Jan Schmidt 2018-07-16 06:39:30 UTC
It's interesting that there's no PTS specifically on the I-frames. It sounds like a quirk of the camera - but as you've noticed will certainly prevent splitmuxsink from detecting those as cut points and working properly.
Comment 4 Jan Schmidt 2018-07-16 14:05:01 UTC
Created attachment 373062 [details] [review]
splitmux: Improve handling of repeated timestamps

When handling input with timestamps that repeat, sometimes
splitmuxsink would get confused and ignore a keyframe.

The logic in question is a holdover from before the cmd queue
moved the file cutting to the multiqueue output side and made
it deterministic, so it's no longer needed on the input
here.
Comment 5 Jan Schmidt 2018-07-16 14:23:06 UTC
For posterity, this example stream when fed through h264parse ends up creating a stream that has stream-format=avc and alignment=au but then has separate SEI-only packets, so it's not in fact alignment=au.

Those caps are what rtph264depay outputs. Is it the stream that's wrong, or does rtph264depay generate the wrong caps?
Comment 6 Tim-Philipp Müller 2018-07-16 14:45:26 UTC
I'm leaning towards rtph264depay behaving wrongly, should it perhaps keep the SEIs around and put them at the beginning of the next AU? This must be specified somewhere surely? Can't really change the output caps unless you always output bytestream.
Comment 7 Jan Schmidt 2018-07-16 15:11:57 UTC
(In reply to Tim-Philipp Müller from comment #6)
> I'm leaning towards rtph264depay behaving wrongly, should it perhaps keep
> the SEIs around and put them at the beginning of the next AU? 

SEI should go with video frames, when alignment=au, yes

> This must be
> specified somewhere surely? Can't really change the output caps unless you
> always output bytestream.

Yes, there's no alignement=nal for AVC
Comment 8 Pedro Corte-Real 2018-07-16 23:08:06 UTC
I can confirm the patch works for one issue (some files being larger than they should) when you also enable PTS interpolation. Should there be a different fix for that? Maybe by enabling interpolation by default?

This also doesn't fix the other issue (first file being a broken small file). From the discussion I understand that issue is actually in rtph264depay doing something broken. Should I open a separate bug for that?
Comment 9 Jan Schmidt 2018-07-16 23:45:08 UTC
Thanks for confirming the patch fixes things for you.

Timestamp interpolation is deliberately disabled in h264parse because it can only create broken timestamps for any H264 stream with packet re-ordering.

This stream is more fundamentally broken - the SEI packets preceding the I-frames carry the timestamps and the I-frame should not be a separate buffer. Having them separated is a violation of the alignment=AU requirements. Muxing that into mp4 can only create dodgy files.

The remaining question is if this is problem with rtph264depay. I suspect it is, and that should be tracked in a separate bug.
Comment 10 Jan Schmidt 2018-07-17 00:58:59 UTC
Pushed to master:

commit f672116c7243a7c3d008b93b686d1e35ddc5b800 (HEAD -> master, origin/master, origin/HEAD)
Author: Jan Schmidt <jan@centricular.com>
Date:   Tue Jul 17 00:03:19 2018 +1000

    splitmux: Improve handling of repeated timestamps
    
    When handling input with timestamps that repeat, sometimes
    splitmuxsink would get confused and ignore a keyframe.
    
    The logic in question is a holdover from before the cmd queue
    moved the file cutting to the multiqueue output side and made
    it deterministic, so it's no longer needed on the input
    here.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=796773

I'd like some more testing before backporting to 1.14. Perhaps for 1.14.3