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 521761 - gstaudioclock frozen the clock value until reaches latest 'last_time'
gstaudioclock frozen the clock value until reaches latest 'last_time'
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal critical
: 0.10.20
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-03-11 12:42 UTC by Edgard Lima
Modified: 2008-05-27 16:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
crete gst_clock_reset function (2.07 KB, patch)
2008-03-11 12:43 UTC, Edgard Lima
none Details | Review
call gst_clock_reset in baseaudio[sink|src] change state funcs (2.29 KB, patch)
2008-03-11 12:44 UTC, Edgard Lima
none Details | Review
call gst_clock_reset in alsaspdifsink change_state (620 bytes, patch)
2008-03-11 12:46 UTC, Edgard Lima
reviewed Details | Review

Description Edgard Lima 2008-03-11 12:42:36 UTC
After change the pipeline state back to NULL,
the 'last_time' value stored in the clock it not reset.

But, the audio base classes reset the number of samples played.

Then when setting the pipeline to PLAYING again, the clock value will be frozen 'cause the clock will always return MAX(last_time, time).

The solution implemented in attached patches do:

1- create a gst_clock_reset funcion
just to make it possible to set last_time to 0 again

2- calls gst_clock_reset, when uses gst_audio_clock is the clock provided, in change_state function from NULL to READY.

I'm just waiting the reviews and OK to commit.

Thanks
Comment 1 Edgard Lima 2008-03-11 12:43:41 UTC
Created attachment 107053 [details] [review]
crete gst_clock_reset function
Comment 2 Edgard Lima 2008-03-11 12:44:37 UTC
Created attachment 107054 [details] [review]
call gst_clock_reset in baseaudio[sink|src] change state funcs
Comment 3 Edgard Lima 2008-03-11 12:46:39 UTC
Created attachment 107055 [details] [review]
call gst_clock_reset in alsaspdifsink   change_state
Comment 4 Wim Taymans 2008-03-11 17:00:25 UTC
Maybe it's better to make sure that the subclass never goes back to 0, this would match the clock API better which says that the get_time() function should be monotonically increasing.
Comment 5 Jan Schmidt 2008-03-11 19:36:15 UTC
Or the sub-class should publish a new clock.

I don't think this is a change we should be making during a freeze. It's not a regression, and not a trivial fix. Feel free to convince me otherwise.
Comment 6 Edgard Lima 2008-03-12 08:42:29 UTC
Jan,

For me it is Ok Jan, for sure it is critical but may be not blocker.
Please change the status to critical and go ahead.

Wim,

ok, I will submit new patches today, then please review it. I will also try to update the documentation, please also review it and point me out to others docs I missed. 
Comment 7 Wim Taymans 2008-05-27 16:20:28 UTC
        * gst-libs/gst/audio/gstaudioclock.c: (gst_audio_clock_init),
        (gst_audio_clock_reset), (gst_audio_clock_get_internal_time):
        * gst-libs/gst/audio/gstaudioclock.h:
        Add method to inform the clock that the time starts from 0 again. We use
        this info to calculate a clock offset so that the time we report in
        internal_time is monotonically increasing, as required by the clock base
        class. Fixes #521761.
        API: GstAudioClock::gst_audio_clock_reset()

        * gst-libs/gst/audio/gstbaseaudiosink.c:
        (gst_base_audio_sink_skew_slaving),
        (gst_base_audio_sink_change_state):
        * gst-libs/gst/audio/gstbaseaudiosrc.c:
        (gst_base_audio_src_create), (gst_base_audio_src_change_state):
        Reset reported time when we (re)create the ringbuffer.
Comment 8 Wim Taymans 2008-05-27 16:32:24 UTC
        * configure.ac:
        Require CVS core and base for new audio clock reset method.

        * ext/alsaspdif/alsaspdifsink.c: (alsaspdifsink_change_state):
        Reset the audio clock. See #521761.