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 775817 - rtph265pay: fix config-interval handling
rtph265pay: fix config-interval handling
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.11.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-12-08 15:21 UTC by Jonas Holmberg
Modified: 2017-01-23 11:09 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Correct handling of config-interval=-1 (21.01 KB, patch)
2016-12-08 15:21 UTC, Jonas Holmberg 		none Details | Review
Correct handling of config-interval (21.32 KB, patch)
2016-12-15 13:57 UTC, Jonas Holmberg 		none Details | Review
rtph265pay: Fix handling of config-interval (21.43 KB, patch)
2017-01-09 13:19 UTC, Jonas Holmberg 		committed Details | Review

Description Jonas Holmberg 2016-12-08 15:21:28 UTC 
Created attachment 341620 [details] [review]
Correct handling of config-interval=-1

When config-interval is set to -1 VPS/SPS/PPS should be inserted before IDR frames, but they are inserted before all NAL units in an access unit (the IDR frame can consist of several NAL unites) and before P-frames.
Comment 1 Jonas Holmberg 2016-12-14 21:07:56 UTC 
I think it was a mistake to not insert VPS/SPS/PPS before P-frames when config-interval > 0. I will update the patch later.

The patch also makes sure VPS/SPS/PPS is not inserted between tiles when config_interval > 0, not just -1 like the commit message subject line reads.
Comment 2 Jonas Holmberg 2016-12-15 13:57:03 UTC 
Created attachment 342028 [details] [review]
Correct handling of config-interval

Same as last patch except it does not prevent instertion of VPS/SPS/PPS before P-frames when config-interval > 0.
Comment 3 Jonas Holmberg 2017-01-09 13:19:02 UTC 
Created attachment 343154 [details] [review]
rtph265pay: Fix handling of config-interval

Insert VPS/SPS/PPS before the first NAL unit containing an I-frame in an
access unit only. If an access unit consists of several such NAL units
(tiles) VPS/SPS/PPS should only be inserted before the first of them so
that parameters are only updated between frames.

Do not insert VPS/SPS/PPS before P-frames when config-interval is -1.
Comment 4 Sebastian Dröge (slomo) 2017-01-09 13:25:27 UTC 
Review of attachment 343154 [details] [review]:

Didn't review it yet, but does this make sure to only insert VPS/SPS/PPS if there is none right before the I frame?
Comment 5 Jonas Holmberg 2017-01-09 14:29:57 UTC 
(In reply to Sebastian Dröge (slomo) from comment #4)
> Review of attachment 343154 [details] [review] [review]:
> 
> Didn't review it yet, but does this make sure to only insert VPS/SPS/PPS if
> there is none right before the I frame?

It always inserts them if config-interval is not 0, just like rtph264pay. So if you have stream-format bytestream you should probably not set config-interval unless you want to duplicate VPS/SPS/PPS.
Comment 6 Sebastian Dröge (slomo) 2017-01-10 14:52:34 UTC 
Just a reminder for myself, the diff should be read with "-w" to ignore all the whitespace. It's mostly indentation changes.
Comment 7 Sebastian Dröge (slomo) 2017-01-23 11:05:37 UTC 
Attachment 343154 [details] pushed as 494246f - rtph265pay: Fix handling of config-interval