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 739620 - input-selector: _activate_sinkpad conflates two different functions
input-selector: _activate_sinkpad conflates two different functions
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 736891
Blocks: 746645
 
 
Reported: 2014-11-04 14:18 UTC by Jan Alexander Steffens (heftig)
Modified: 2015-03-24 13:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch 1/3 (1.14 KB, patch)
2014-11-19 12:47 UTC, Jan Alexander Steffens (heftig)
none Details | Review
patch 2/3 (1.49 KB, patch)
2014-11-19 12:47 UTC, Jan Alexander Steffens (heftig)
none Details | Review
patch 3/3 (4.82 KB, patch)
2014-11-19 12:48 UTC, Jan Alexander Steffens (heftig)
none Details | Review
patch 1/3 (1.14 KB, patch)
2015-03-23 13:31 UTC, Jan Alexander Steffens (heftig)
committed Details | Review
patch 2/3 (1.49 KB, patch)
2015-03-23 13:31 UTC, Jan Alexander Steffens (heftig)
committed Details | Review
patch 3/3 (5.19 KB, patch)
2015-03-23 13:31 UTC, Jan Alexander Steffens (heftig)
committed Details | Review

Description Jan Alexander Steffens (heftig) 2014-11-04 14:18:35 UTC
Seems input-selector contains a lot of bit-rot.

gst_input_selector_activate_sinkpad is used to:

1) get the current active sink pad, falling back to the first existing pad,
2) Set the "active" field on the given pad to TRUE.

The function 2) seems to be a remnant from when _activate_sinkpad still used the given pad as the fallback pad. This function should be split into two separate functions.

Also, it seems the "active" field of the sink pad is just used to determine whether the running-time property returns 0.

However, since this field is set on any activity it cannot be used to determine whether gst_segment_to_running_time works or not. This should probably be changed to check if the segment is defined and return GST_CLOCK_TIME_NONE if it is not.

In addition, Bug 736891 is something to clean up. The block signal can only be undone by state-change.
Comment 1 Jan Alexander Steffens (heftig) 2014-11-19 12:47:13 UTC
Created attachment 290985 [details] [review]
patch 1/3
Comment 2 Jan Alexander Steffens (heftig) 2014-11-19 12:47:37 UTC
Created attachment 290986 [details] [review]
patch 2/3
Comment 3 Jan Alexander Steffens (heftig) 2014-11-19 12:48:00 UTC
Created attachment 290987 [details] [review]
patch 3/3
Comment 4 Jan Alexander Steffens (heftig) 2015-03-10 10:15:36 UTC
Please review the patches (they are on top of the patch for bug 736891).
Comment 5 Sebastian Dröge (slomo) 2015-03-23 10:31:17 UTC
I'll merge them once bug 736891 is done
Comment 6 Jan Alexander Steffens (heftig) 2015-03-23 13:31:05 UTC
Created attachment 300132 [details] [review]
patch 1/3

Rebased to master (and bug 736891).
Comment 7 Jan Alexander Steffens (heftig) 2015-03-23 13:31:27 UTC
Created attachment 300133 [details] [review]
patch 2/3
Comment 8 Jan Alexander Steffens (heftig) 2015-03-23 13:31:47 UTC
Created attachment 300134 [details] [review]
patch 3/3
Comment 9 Thiago Sousa Santos 2015-03-24 13:46:39 UTC
Pushed, thanks.

commit 27644a6bd2ca96cf6e02a7508c31e89d5389113b
Author: Jan Alexander Steffens (heftig) <jsteffens@make.tv>
Date:   Wed Nov 19 13:08:45 2014 +0100

    input-selector: Rename _activate_sinkpad to _get_active_sinkpad
    
    Removes the now unused 'pad' parameter and renames the function
    to something more appropriate.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=739620

commit bf8a71104c70faab0cf03ab94f4fd94bb22e5e1f
Author: Jan Alexander Steffens (heftig) <jsteffens@make.tv>
Date:   Wed Nov 19 13:03:21 2014 +0100

    input-selector: Remove pad's 'active' field
    
    This is now never read.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=739620

commit 6f24f4917d6cdde5cdb6935f3ff6eb21948c714b
Author: Jan Alexander Steffens (heftig) <jsteffens@make.tv>
Date:   Wed Nov 19 12:59:12 2014 +0100

    input-selector: Use segment-presence for running_time check
    
    When determining whether the running_time of a pad can be
    calculated, check if the segment is in TIME format instead
    of using the 'active' field.
    
    Since the latter is set through *any* activity, it's not a
    reliable indicator of segment presence.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=739620