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 791495 - h265parse: Messes up timestamps for byte-stream
h265parse: Messes up timestamps for byte-stream
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other All
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-12-12 00:22 UTC by Matej Knopp
Modified: 2018-11-03 14:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Experimental patch (1.07 KB, patch)
2017-12-12 00:47 UTC, Matej Knopp
none Details | Review
Better patch (1.69 KB, patch)
2017-12-12 13:24 UTC, Matej Knopp
none Details | Review

Description Matej Knopp 2017-12-12 00:22:37 UTC
It happens when previous frame has trailing zero. Parser split frame too early, leaving trailing zeros as prefix for new frame, which results in messed up timestamps.

Caused by following code in gst_h265_parser_identify_nalu

  /* Mini performance improvement:
   * We could have a way to store how many 0s were skipped to avoid
   * parsing them again on the next NAL */
  while (off2 > 0 && data[nalu->offset + off2 - 1] == 00)
    off2--;

Annex B does says that there should be zero byte before AU startcode, however it also says that during parsing the zero byte should be discarded. So maybe determining the timestamp from zero byte is not a very good idea.

The stream already parsed by FFMPEG HEVC parser, which seems to leave zero byte trailing, but that doesn't mean such files can't be encountered in the wild.

Btw. Why's there loop? Why skipping more than 1 zero byte? Doesn't that just increases chance of messed up timestamps?
Comment 1 Tim-Philipp Müller 2017-12-12 00:31:24 UTC
Maybe you could make a unit test for this? :)
Comment 2 Matej Knopp 2017-12-12 00:41:18 UTC
Okay, the loop doesn't make any difference because of the following code

       if (nalu.sc_offset > 0) {
          *skipsize = nalu.sc_offset;

So perhaps changing this to

        if (nalu.offset > 3) {
          *skipsize = nalu.offset - 3;

would discard the zero_byte, which is what we should do according annex b?
Comment 3 Matej Knopp 2017-12-12 00:47:14 UTC
Created attachment 365396 [details] [review]
Experimental patch

I agree unit test would be nice, but for now at least experimental patch, seems to fix the situation for me
Comment 4 Matej Knopp 2017-12-12 00:55:38 UTC
Well, while working this patch probably also introduces DISCONT buffers.
Comment 5 Matej Knopp 2017-12-12 13:24:37 UTC
Created attachment 365425 [details] [review]
Better patch

Inspired by h264parse, should not cause unnecessary discont buffers
Comment 6 Nicolas Dufresne (ndufresne) 2018-10-13 19:46:18 UTC
I notice this difference between H264/H265parse and was wondering why we did that extra loop. Make sense.
Comment 7 GStreamer system administrator 2018-11-03 14:16:32 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/641.