GNOME Bugzilla – Bug 793284
h264parses: Expose framerate even if the fixed_frame_rate flag isn't set
Last modified: 2018-08-27 10:04:11 UTC
Created attachment 368101 [details] Dumped SPS of the sample stream I've came accross a test H264 byte-stream and was surprised to find out the ffmpeg could make up a framerate where GStreamer could not. I've attached the SPS dump, one can notice that fixed_frame_rate_flag is not set. After going through the spec, it seems that this flags has an implication on the decoder processing of interleaved content, but it's not stated anywhere that the framerate value is not valid. All in all, I think FFMPEG is right, we should consider this framerate.
Created attachment 368102 [details] [review] h264parser: Expose framerate even if fixed_frame_rate flag isn't set There is nothing in the spec that state that framerate is not valid in that case. This aligns GStreamer with FFMPEG behaviour for similar streams.
Attachment 368102 [details] pushed as 4300611 - h264parser: Expose framerate even if fixed_frame_rate flag isn't set
Created attachment 368740 [details] [review] baseparse: Fix check for update_interval update_interval may be -1
Created attachment 368741 [details] [review] baseparse: Avoid overflow in update_interval calculation
Created attachment 368742 [details] [review] baseparse: Fix integer overflow in bitrate calculation
Those 3 patches against baseparse fixes buffer overflows that would lead to assertions. It fixes gst-plugins-bad elements_h264parse unit test that was failing after this change. The reason is that the test SPS contains a really high framerate (1000000000 fps). I think it's a good test, so I left it like this. I'll review myself tomorrow before merging, other's review is of course welcome.
Attachment 368740 [details] pushed as 25aed8c - baseparse: Fix check for update_interval Attachment 368741 [details] pushed as a84b886 - baseparse: Avoid overflow in update_interval calculation Attachment 368742 [details] pushed as 91798e1 - baseparse: Fix integer overflow in bitrate calculation
Created attachment 368861 [details] [review] gl: Add meson support for GBM backend https://bugzilla.gnome.org/show_bug.cgi?id=782923
(In reply to Nicolas Dufresne (stormer) from comment #8) > Created attachment 368861 [details] [review] [review] > gl: Add meson support for GBM backend > > https://bugzilla.gnome.org/show_bug.cgi?id=782923 Wrong bug ? :)
Outch
Review of attachment 368742 [details] [review]: ::: libs/gst/base/gstbaseparse.c @@ +1819,3 @@ + 8 * parse->priv->data_bytecount, parse->priv->acc_duration); + + if (avg_bitrate > G_MAXUINT) Shouldn't this be a G_MAXINT? avg_bitrate is a gint, not guint. This is causing an assert here in with the following command: youtube-dl "https://vimeo.com/280481143" -o - | G_DEBUG=fatal_criticals gst-play-1.0 fd://0 (gdb) bt
+ Trace 238678