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 796491 - gstplayer: CRITICAL assertion on duration change when playing audio files
gstplayer: CRITICAL assertion on duration change when playing audio files
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.14.0
Other Linux
: Normal normal
: 1.14.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-06-04 06:52 UTC by Lyon
Modified: 2018-06-21 10:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
flac audio clip for test (854.37 KB, audio/flac)
2018-06-04 06:52 UTC, Lyon
  Details
patch for fixing duration critical assertion. (1.37 KB, patch)
2018-06-04 08:42 UTC, Lyon
none Details | Review
Updated patch for fix critical warning issue (981 bytes, patch)
2018-06-04 09:46 UTC, Lyon
committed Details | Review

Description Lyon 2018-06-04 06:52:05 UTC
Created attachment 372534 [details]
flac audio clip for test

Hi,
   We found some duration change CRITICAL print when playing audio files.

CRITICAL **: emit_duration_changed: assertion 'self->cached_duration != duration' failed
   Our gplay app is based on gstplayer API, this assertion is in emit_duration_changed() in gstplayer.c  the cached_duration is the same as new duration.

 For the flac audio files (actually all the flac files on hand), when we were trying to FB ( say set rate to -1), 
it will report this CRITICAL warning.
  
I had some debugging and found that in gst_base_parse_loop(): 
    if (push_eos) {
      if (parse->priv->estimated_duration <= 0) {
          GST_ERROR("gst_base_parse_update_duration");
        gst_base_parse_update_duration (parse);
      }
     ...
    }

    When change rate (need flushing),the estimated_duration here is -1 (then in gst_base_parse_update_duration() , the estimated_drift is large), so it will try to update the duration and post duration change message. 
    However, the queried duration is the same as cached_duration, which cause this issue. 
    I see above code is for fixing small mp3 audio duration issue (https://bugzilla.gnome.org/show_bug.cgi?id=750131)


    So do you have any suggestion for flac audio clips rewind CRITICAL assertion issue?
Comment 1 Sebastian Dröge (slomo) 2018-06-04 07:42:37 UTC
That assertion should be changed into an early return (i.e. if nothing changes, don't signal). Do you want to provide a patch for this?
Comment 2 Lyon 2018-06-04 08:08:16 UTC
Yes, that's what I can think of. If nothing changes, we don't signal

I will provide a patch for this, thanks, slomo.
Comment 3 Lyon 2018-06-04 08:42:18 UTC
Created attachment 372536 [details] [review]
patch for fixing duration critical assertion.

Please see attachment for the patch
Comment 4 Sebastian Dröge (slomo) 2018-06-04 09:20:02 UTC
Review of attachment 372536 [details] [review]:

::: gst-libs/gst/player/gstplayer.c
@@ +1701,2 @@
   if (gst_element_query_duration (self->playbin, GST_FORMAT_TIME, &duration)) {
+    if (self->cached_duration !=  duration)

There's a double-space here and in the other place before duration.

Also I meant putting this comparison inside emit_duration_changed() and just doing an early return there instead of asserting :)
Comment 5 Lyon 2018-06-04 09:46:49 UTC
Created attachment 372537 [details] [review]
Updated patch for fix critical warning issue
Comment 6 Sebastian Dröge (slomo) 2018-06-04 18:07:33 UTC
commit 92576e7db83a548fcb54e80e35758a7c10e08cf2 (HEAD -> master)
Author: Lyon Wang <lyon.wang@nxp.com>
Date:   Mon Jun 4 16:35:41 2018 +0800

    player: Fix duration-changed CRITICAL warning if duration did not actually change
    
    Check if duration is changed before emitting duration-changed signal
    
    https://bugzilla.gnome.org/show_bug.cgi?id=796491