GNOME Bugzilla – Bug 754124
h265parse/h264parse: Resync these two parsers
Last modified: 2018-09-11 20:09:59 UTC
There seems to have been updates on h264 that never got applied to h265. This is often a bad sign for the state of h265parse element. Please sync these two, or create a common base class for it.
I can look into this :)
(In reply to Nicolas Dufresne (stormer) from comment #0) > There seems to have been updates on h264 that never got applied to h265. > This is often a bad sign for the state of h265parse element. Please sync > these two, or create a common base class for it. Thought about this many times, didn't get time to implement though :) solution1 (sync) :is easier, you can easily take the patches from here:https://github.com/01org/gstreamer-codecparsers (we are keeping h264parser and h265parser in videoparsers folder, always in sync with upstream, didn't miss any patches i guess) solution2 (base class for h26x): this is more elegant :) --
sreerenj, I don't understand. You are suggesting that we grab patches from a repo that looks like is in sync with gst-plugins-bad and doesn't have any new commits on-top of it. https://github.com/01org/gstreamer-codecparsers/commits/master/gst/videoparsers/gsth265parse.c Am I missunderstanding you?
(In reply to Luis de Bethencourt from comment #3) > sreerenj, > > I don't understand. You are suggesting that we grab patches from a repo that > looks like is in sync with gst-plugins-bad and doesn't have any new commits > on-top of it. Yes, you can easily see the patches which landed only in h264parser (and not in h265parser) ...There are no new commits other than the upstream patches. I usually cherry-pick patches from upstream... We had some custom patches, but that is inside gstreamer-vaapi (gstreamer-vaapi/patches/videoparsers/).. But those are not needed in upstream :) > > https://github.com/01org/gstreamer-codecparsers/commits/master/gst/ > videoparsers/gsth265parse.c > > Am I missunderstanding you?
(In reply to sreerenj from comment #4) > > Yes, you can easily see the patches which landed only in h264parser (and not > in h265parser) ...There are no new commits other than the upstream patches. > I usually cherry-pick patches from upstream... > > We had some custom patches, but that is inside gstreamer-vaapi > (gstreamer-vaapi/patches/videoparsers/).. But those are not needed in > upstream :) > I was planning to use: git log -p gst/videoparsers/gsth265parse.c and similar. Thanks for the suggestion :)
Of course, one does not prevent the other.
Can I update resync patches in here? There are some bugs in h265parse which could be fixed by resync patches.
> Can I update resync patches in here? There are some bugs in h265parse which > could be fixed by resync patches. Yes please, by all means! :)
Created attachment 373310 [details] [review] [1/12] h265parse: Copy over DISCONT flag from input buffers Apply the commit 10ffa08
Created attachment 373311 [details] [review] [2/12] h265parse: Unref/replace force_key_unit_event in gst_h265_parse_reset Apply the commit 36a2aca
Created attachment 373312 [details] [review] [3/12] h265parse: Fix and optimize NAL collection function Adopt h264parse's _collect_nal() behavior. See also commit 5601c87 and https://bugzilla.gnome.org/show_bug.cgi?id=732154
Created attachment 373313 [details] [review] [4/12] h265parse: Introduce new state tracking variables Direct applying the commit 7bb6443. This could fix also unexpected nal dropping when nonzero "config-interval" is set. (e.g., gst-launch-1.0 videotestsrc ! x265enc key-int-max=30 ! h265parse config-interval=30 ! avdec_h265 ! videoconvert ! autovideosink) Similar to the h264parse, have_{vps,sps,pps} variables will be used for deciding on when to submit updated caps or not, and rather mean "have new SPS/PPS to be submitted?" See also https://bugzilla.gnome.org/show_bug.cgi?id=732203
Created attachment 373314 [details] [review] [5/12] h265parse: Improve conditions for skipping NAL units See also https://bugzilla.gnome.org/show_bug.cgi?id=732203
Created attachment 373315 [details] [review] [6/12] h265parse: Fix collection of access units to preserve config headers Apply the commit 7d44a51 See also https://bugzilla.gnome.org/show_bug.cgi?id=732203
Created attachment 373316 [details] [review] [7/12] h265parse: Reset the parser information when caps changes Apply the commit 14f6fcd
Created attachment 373317 [details] [review] [8/12] h265parse: Don't unref buffer that was unreffed just a few lines before already Apply the commit 9b50a12
Created attachment 373318 [details] [review] [9/12] h265parse: Consider SEI NALU as "HEADER" packets Apply the commit 69c09c3
Created attachment 373319 [details] [review] [10/12] h265parse: Don't discard first AU delimiter Apply the commit 48a1f27
Created attachment 373320 [details] [review] [11/12] h265parse: Add support insert parameter set per IDR Apply commits 0c04e00, bf0d952 and a0876aa to h265parse. See also https://bugzilla.gnome.org/show_bug.cgi?id=766803
Created attachment 373321 [details] [review] [12/12] h265parse: Fix periodic SPS/PPS sending work after a seek Apply the commit ef71b61 See also https://bugzilla.gnome.org/show_bug.cgi?id=742212
Thanks for the patches, this is greatly appreciated, and needed. Event if we go a head and do comment 2 solution 2 (Base class) we would have had to do that first, and this will fix real life bugs indeed.
Because git-bz didn't work: remote: sending e-mail for 7032b6a454bef6df5cb8f8dd3e766ba96f051347 remote: sending e-mail for 2bd883621fb4ead4d2c1e85fc400f32c1766f8ab remote: sending e-mail for ad7cf957fb3209d875e7b78e82dc617ecc37af23 remote: sending e-mail for c0756d0909a372933dd2847007af27a4216f1105 remote: sending e-mail for ababccbcb2ea3360cce69da8e069cfb414e64fd1 remote: sending e-mail for 80cab68889696084be30970e93663abe915cbdc6 remote: sending e-mail for 27432ab0672c8546f5fb626786589b406a61f689 remote: sending e-mail for 3ad30ef76e67f88417f782578772540a90c918cc remote: sending e-mail for 60d8b7184fd188304712724bce489863a03f2d6e remote: sending e-mail for 8b57392b92a9da93a4c28444e91249928bd5c43f remote: sending e-mail for fd79d8d7a36278fd415f89c10632302430720b1a remote: sending e-mail for da7143078f35c48a392e4f7d320a135169010ff4
A lot of these H264 patches are in stable releases. So this is worth considering for 1.14.3.
(In reply to Nicolas Dufresne (ndufresne) from comment #21) > Thanks for the patches, this is greatly appreciated, and needed. Event if we > go a head and do comment 2 solution 2 (Base class) we would have had to do > that first, and this will fix real life bugs indeed. Is there someone who is working on h26xbaseparse?
Not that I know of. But considering your work, it's clear it would be worth it. Best is to dedicate a bug for this enhancement.
Backported everything to 1.14. remote: sending e-mail for e47b013804b81e8d6719563232376badde0a64e3 remote: sending e-mail for 0d61e1c485be29c3f318bfceb5f6fe053f0c97ab remote: sending e-mail for d3ee00ec984ff81240bc723af72863a45ac85870 remote: sending e-mail for 37bb4cec356a06ac7e4a266fd52c7241892e1001 remote: sending e-mail for fe4d898d701be1febed01177c6cbac2fcad5a920 remote: sending e-mail for cedd8e125e1fcdaab48dd1f7f6528f81acd1a79b remote: sending e-mail for 42f324cb91a4bb51923d9f281ba17d70e3eeed86 remote: sending e-mail for 3c5b6c03f49b841c19ae26fc07e05100c451806a remote: sending e-mail for f652829d5fef9f36a3e06010753b791612140c54 remote: sending e-mail for c40c4b5fc38a201ed27ec785092794acb64518b4 remote: sending e-mail for 06367b4c86d7a059a48bb430e04f89a28f06c426 remote: sending e-mail for 594246a6400e81007a7cccc2721b4d1a4b00729c