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 621190 - video sink drops buffers if it's preceded by ffmpegcolorspace, videoscale and a capsfilter
video sink drops buffers if it's preceded by ffmpegcolorspace, videoscale and...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.30
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-06-10 11:16 UTC by Philippe Normand
Modified: 2010-06-18 08:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[ximagesink] Ask pad peer to accept new caps once only (5.58 KB, patch)
2010-06-14 17:30 UTC, Philippe Normand
none Details | Review
[ximagesink] Ask pad peer to accept new caps once only (5.53 KB, patch)
2010-06-14 17:37 UTC, Philippe Normand
none Details | Review
[ximagesink] Ask pad peer to accept new caps once only (5.87 KB, patch)
2010-06-15 13:28 UTC, Philippe Normand
none Details | Review
[ximagesink] Ask pad peer to accept new caps once only (5.81 KB, patch)
2010-06-17 13:39 UTC, Philippe Normand
committed Details | Review

Description Philippe Normand 2010-06-10 11:16:59 UTC
Try this and put the window in fullscreen. The sink will drop buffers:

gst-launch videotestsrc pattern=1 ! ffmpegcolorspace ! videoscale  ! "video/x-raw-rgb,width=(int)500,height=(int)250" ! ximagesink

If ffmpegcolorspace is removed from the pipeline everything runs smoothly.
Comment 1 Wim Taymans 2010-06-14 09:39:19 UTC
ximagesink keeps on suggesting the new caps on its sinkpad over and over again which causes a lot of caps negotiation. 

What it should do is check if the new caps are accepted downstream once and if they aren't, never try to suggest these same caps again.
Comment 2 Wim Taymans 2010-06-14 10:48:36 UTC
These two patches already solve it but it would be nice if ximagesink also didn't hammer the acceptcaps function of upstream elements.

commit d612442fde5f8c55d59de43fc2a1a9d9a4037c10
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Jun 14 12:39:47 2010 +0200

    capsfilter: implement custom accept_caps method
    
    Implement a custom acceptcaps function. We can simply check if there is an
    intersection with the new caps. This makes the accept caps function much faster.
    
    See #621190

commit 76f7a001fc4978c0aa5d202be38ca5943ae671ed
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Jun 14 12:36:54 2010 +0200

    basetransform: add accept_caps vmethod
    
    Allow subclasses to override the acceptcaps function because in some cases a
    custom implementation can be much much faster than the default one.
    
    See #621190
Comment 3 Philippe Normand 2010-06-14 17:30:28 UTC
Created attachment 163614 [details] [review]
[ximagesink] Ask pad peer to accept new caps once only

In buffer_alloc, if the buffer dimensions are different than the
window dimensions, call gst_pad_peer_accept_caps once only, it's
useless to call it in the cases where we know it will always fail.

Fixes bug #621190
Comment 4 Philippe Normand 2010-06-14 17:37:54 UTC
Created attachment 163615 [details] [review]
[ximagesink] Ask pad peer to accept new caps once only

In buffer_alloc, if the buffer dimensions are different than the
window dimensions, call gst_pad_peer_accept_caps once only, it's
useless to call it in the cases where we know it will always fail.

Fixes bug #621190
Comment 5 Sebastian Dröge (slomo) 2010-06-15 09:57:25 UTC
Instead of just checking for width/height changes you should probably check for equal caps. xvimagesink does it this way.
Comment 6 Philippe Normand 2010-06-15 13:28:12 UTC
Created attachment 163674 [details] [review]
[ximagesink] Ask pad peer to accept new caps once only

[ximagesink] Ask pad peer to accept new caps once only

In buffer_alloc, if the buffer caps are new, call
gst_pad_peer_accept_caps once only, it's useless to call it in the
cases where we know it will always fail.

Fixes bug #621190
Comment 7 Sebastian Dröge (slomo) 2010-06-15 13:58:37 UTC
I don't think this is correct, you're checking if the last_caps are set and for equality while you should check if the last caps are *not* set or they are inequal
Comment 8 Sebastian Dröge (slomo) 2010-06-15 14:00:19 UTC
Note that this also breaks your above pipeline if you try to resize the ximagesink video... this will result in a not-negotiated error after your patch (and my proposed changes from comment #7) and before your patch will resize the window and add black borders to the video.
Comment 9 Philippe Normand 2010-06-15 14:30:25 UTC
(In reply to comment #8)
> Note that this also breaks your above pipeline if you try to resize the
> ximagesink video... this will result in a not-negotiated error after your patch
> (and my proposed changes from comment #7) and before your patch will resize the
> window and add black borders to the video.

Right... will check this and do more tests, sorry about the noise :/
Comment 10 Philippe Normand 2010-06-17 13:39:29 UTC
Created attachment 163913 [details] [review]
[ximagesink] Ask pad peer to accept new caps once only

In buffer_alloc, if the buffer caps are new, call
gst_pad_peer_accept_caps once only, it's useless to call it in the
cases where we know it will always fail.

Fixes bug #621190
Comment 11 Sebastian Dröge (slomo) 2010-06-18 04:13:48 UTC
commit 0ee588a3a6bfb0f0e89fd8f74757b0045b98c923
Author: Philippe Normand <pnormand@igalia.com>
Date:   Mon Jun 14 12:27:02 2010 +0200

    ximagesink: Ask pad peer to accept new caps once only
    
    In buffer_alloc, if the buffer caps are new, call
    gst_pad_peer_accept_caps once only, it's useless to call it in the
    cases where we know it will always fail.
    
    Fixes bug #621190