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 757688 - rtponviftimestamp: element does not work properly
rtponviftimestamp: element does not work properly
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-11-06 14:52 UTC by Ognyan Tonchev (redstar_)
Modified: 2015-11-09 09:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rtponvif: split unit tests in several files (30.15 KB, patch)
2015-11-06 14:53 UTC, Ognyan Tonchev (redstar_)
committed Details | Review
rtponviftimestamp: Do not rearange order of data (23.30 KB, patch)
2015-11-06 14:54 UTC, Ognyan Tonchev (redstar_)
committed Details | Review
rtponviftimestamp: Update ntp-offset and d/e-bits with a GstEvent (26.43 KB, patch)
2015-11-06 14:55 UTC, Ognyan Tonchev (redstar_)
committed Details | Review
rtponviftimestamp: use stream time for timestamp (5.78 KB, patch)
2015-11-06 14:56 UTC, Ognyan Tonchev (redstar_)
committed Details | Review

Description Ognyan Tonchev (redstar_) 2015-11-06 14:52:09 UTC
Setting ntp offset with a property is not good enough. Setting an offset in state other than NULL is not allowed but even if it was it would be racy. We basically need to know from which buffer the new offset is to be applied.

Another issue is that the discont flag on the buffer can't be used for setting the E/D-bits. The discont flag in a buffer can be set whenever a live/network source looses a frame, but that is not the type of discontinuity that the onvif extension header should reflect. The header is mainly used for playback of a track concept, in which gaps can be present, and it's those kind of gaps that should be highlighted with the D- and E-bits.

The element currently uses running time for Onvif-timestamping. The Onvif Streaming Specification specifies that the NTP timestamps in the Onvif extension header indicaes the absolute UTC time associated with the access unit. But by using running time we can not achieve that, since a frame's running time depends on the played interval, whether a non-flushing is done, etc.

A bug that we found as well and have now fixed: If a buffer or a buffer list is cached, no events serialized with the data stream should get through. The cached buffers and events should be purged when we stop flushing.

There is also a small patch attached which splits unit tests in several files(there are two separate elements and we now have one file per element) and makes it work with CK_FORK=no and in valgrind.
Comment 1 Ognyan Tonchev (redstar_) 2015-11-06 14:53:56 UTC
Created attachment 314994 [details] [review]
rtponvif: split unit tests in several files

Split the unit tests for rtponviftimestamp and rtponvifparse
elements in separate files.
Setup and cleanup the element and pads in fixures. Make the tests work
with CK_FORK=no as well, by cleaning up the 'buffers' list when needed.
Make unit tests work when run in valgrind by unreffing all buffers,
and by not allocating any payload in RTP buffers. Since we're not
doing anything with the payload part, but we're memcmp-aring the
complete buffer memory, valgrind complained about non-initialized
memory being used.
Comment 2 Ognyan Tonchev (redstar_) 2015-11-06 14:54:29 UTC
Created attachment 314995 [details] [review]
rtponviftimestamp: Do not rearange order of data

If a buffer or a buffer list is cached, no events serialized with the
data stream should get through. The cached buffers and events should
be purged when we stop flushing.
Comment 3 Ognyan Tonchev (redstar_) 2015-11-06 14:55:25 UTC
Created attachment 314996 [details] [review]
rtponviftimestamp: Update ntp-offset and d/e-bits with a GstEvent

It is now possible to update the currently used ntp-offset with a
custom serialized downstream event. The element will read the ntp-offset
property when doing the state transition from READY to PAUSED and
use that offset until it receives a "GstNtpOffset" event, which also
has a "ntp-offset" attribute in that it's structure. In case the
property is not set and no event has been received, the element will
guess the npt-offset with help of the clock. If no clock can be
retrieved, the element will error out and stop the data flow.
    
The same event is also used for updating the D/E-bits in the RTP
extension header. The discont flag in a buffer can be set whenver a
live/network source looses a frame, but that is not the type of
discontinuity that the onvif extension header should reflect. The
header is mainly used for playback of a track concept, in which
gaps can be present, and it's those kind of gaps that should be
highlighted with the D- and E-bits.
Comment 4 Ognyan Tonchev (redstar_) 2015-11-06 14:56:05 UTC
Created attachment 314997 [details] [review]
rtponviftimestamp: use stream time for timestamp

The Onvif Streaming Specification specifies that the NTP timestamps
in the Onvif extension header indicaes the absolute UTC time associated
with the access unit. But by using running time we can not achieve that,
since a frame's running time depends on the played interval, whether a
non-flushing is done, etc. Instead we have to use the stream time.
Comment 5 Ognyan Tonchev (redstar_) 2015-11-06 14:56:53 UTC
Patches above solve these issues and they should be applied in the order they are submitted.
Comment 6 Olivier Crête 2015-11-06 18:08:42 UTC
Merged, thank you!


commit 4c482befbd339b626d598e20bf52c1f536564c36
Author: Branko Subasic <branko@axis.com>
Date:   Thu Oct 22 13:40:36 2015 +0200

    rtponviftimestamp: use stream time for timestamp
    
    The Onvif Streaming Specification specifies that the NTP timestamps
    in the Onvif extension header indicaes the absolute UTC time associated
    with the access unit. But by using running time we can not achieve that,
    since a frame's running time depends on the played interval, whether a
    non-flushing is done, etc. Instead we have to use the stream time.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=757688

commit 6f6fe37ed995f4631859414a08e15f35858d29c9
Author: Linus Svensson <linussn@axis.com>
Date:   Fri Nov 6 09:44:57 2015 +0100

    rtponviftimestamp: Update ntp-offset and d/e-bits with a GstEvent
    
    It is now possible to update the currently used ntp-offset with a
    custom serialized downstream event. The element will read the ntp-offset
    property when doing the state transition from READY to PAUSED and
    use that offset until it receives a "GstNtpOffset" event, which also
    has a "ntp-offset" attribute in that it's structure. In case the
    property is not set and no event has been received, the element will
    guess the npt-offset with help of the clock. If no clock can be
    retrieved, the element will error out and stop the data flow.
    
    The same event is also used for updating the D/E-bits in the RTP
    extension header. The discont flag in a buffer can be set whenver a
    live/network source looses a frame, but that is not the type of
    discontinuity that the onvif extension header should reflect. The
    header is mainly used for playback of a track concept, in which
    gaps can be present, and it's those kind of gaps that should be
    highlighted with the D- and E-bits.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=757688

commit a58826292eab80e83b9de525753895e56410e301
Author: Linus Svensson <linussn@axis.com>
Date:   Fri Nov 6 09:44:16 2015 +0100

    rtponviftimestamp: Do not rearange order of data
    
    If a buffer or a buffer list is cached, no events serialized with the
    data stream should get through. The cached buffers and events should
    be purged when we stop flushing.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=757688

commit fd0ca0a9723931463c8be93517d13b5ef799f6f0
Author: Branko Subasic <branko@axis.com>
Date:   Tue Oct 13 14:21:47 2015 +0200

    rtponvif: split unit tests in several files
    
    Split the unit tests for rtponviftimestamp and rtponvifparse
    elements in separate files.
    Setup and cleanup the element and pads in fixures. Make the tests work
    with CK_FORK=no as well, by cleaning up the 'buffers' list when needed.
    Make unit tests work when run in valgrind by unreffing all buffers,
    and by not allocating any payload in RTP buffers. Since we're not
    doing anything with the payload part, but we're memcmp-aring the
    complete buffer memory, valgrind complained about non-initialized
    memory being used.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=757688
Comment 7 Olivier Crête 2015-11-06 18:09:05 UTC
Out of curiosity, how do you produce the custom event?
Comment 8 Linus Svensson 2015-11-09 09:27:48 UTC
Hi,

We produce the events in a custom source element. The source element puts video clips together into one stream, where any gaps between the clips are removed. This source element also know which absolute time the clips was produced.