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 753360 - videodecoder: pushes gap before segment
videodecoder: pushes gap before segment
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal normal
: 1.5.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-08-07 15:44 UTC by Thiago Sousa Santos
Modified: 2015-08-16 13:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
videodecoder: push pending events before gap (6.31 KB, patch)
2015-08-10 03:31 UTC, Thiago Sousa Santos
committed Details | Review
tests: audiodecoder: add test to make sure gap is pushed before segment (2.69 KB, patch)
2015-08-10 03:31 UTC, Thiago Sousa Santos
committed Details | Review

Description Thiago Sousa Santos 2015-08-07 15:44:23 UTC
If the videodecoder receives a gap before buffers, the gap event is pushed before the segment itself. The timestamp on the gap has no real running-time without an associated segment.

One of the problems on fixing this is that the caps event must be pushed before the segment and some decoders would only know the complete caps after decoding a frame.

Holding the gap until a buffer is received would cause issues in prerolling so I'm not sure it is a good idea.

Maybe sending incomplete caps and later updating it?

(Haven't checked the audiodecoder but it might have the same issue)
Comment 1 Sebastian Dröge (slomo) 2015-08-08 09:40:46 UTC
There is code in videodecoder to generate some dummy caps, see gst_video_decoder_negotiate_default_caps() in the GST_EVENT_GAP handler. I think the only thing missing in that code then would be something to also send a SEGMENT event before forwarding the GAP. The SEGMENT event to be sent should be in current_frame_events
Comment 2 Thiago Sousa Santos 2015-08-08 14:08:07 UTC
  GST_EVENT_SEGMENT
  GST_EVENT_TAG
  GST_EVENT_BUFFERSIZE
  GST_EVENT_SINK_MESSAGE 
  GST_EVENT_EOS
  GST_EVENT_TOC
  GST_EVENT_PROTECTION
  GST_EVENT_SEGMENT_DONE
  GST_EVENT_GAP

I guess we can push all events before the gap?
Comment 3 Sebastian Dröge (slomo) 2015-08-08 14:22:48 UTC
I think so, yes. But only after draining the decoder
Comment 4 Thiago Sousa Santos 2015-08-10 03:31:11 UTC
Created attachment 308991 [details] [review]
videodecoder: push pending events before gap

Push all pending events before pushing the gap. This ensures the
segment is pushed before the gap so it can be properly translated
to the running time

Includes unit test.
Comment 5 Thiago Sousa Santos 2015-08-10 03:31:16 UTC
Created attachment 308992 [details] [review]
tests: audiodecoder: add test to make sure gap is pushed before segment
Comment 6 Sebastian Dröge (slomo) 2015-08-10 08:46:55 UTC
Comment on attachment 308992 [details] [review]
tests: audiodecoder: add test to make sure gap is pushed before segment

Typo in commit message (gap before segment vs segment before gap)

audiodecoder already does the right thing? Unexpected :)
Comment 7 Sebastian Dröge (slomo) 2015-08-10 08:47:27 UTC
Comment on attachment 308991 [details] [review]
videodecoder: push pending events before gap

If you want to make Tim happy, also convert from GList to GQueue in another commit :)
Comment 8 Thiago Sousa Santos 2015-08-10 11:06:44 UTC
Bug fixed, will try to increase Tim's happiness level in a follow up commit :)

Audiodecoder already worked because it handles gaps by using empty buffers.

(Sorry, forgot to fix the commit message.)


commit 30e9c26b729188ca0bd7ddcc83ab49ff27595c71
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Mon Aug 10 00:21:42 2015 -0300

    tests: audiodecoder: add test to make sure gap is pushed before segment
    
    https://bugzilla.gnome.org/show_bug.cgi?id=753360

commit e59d1308cc55da6cca949d9d954e32d3f3d76343
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Sun Aug 9 23:23:05 2015 -0300

    videodecoder: push pending events before gap
    
    Push all pending events before pushing the gap. This ensures the
    segment is pushed before the gap so it can be properly translated
    to the running time
    
    Includes unit test.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=753360