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 616422 - playsink might not handle reconfiguring after a text enabled file correctly
playsink might not handle reconfiguring after a text enabled file correctly
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal normal
: 0.10.30
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-04-21 18:47 UTC by Julien MOUTTE
Modified: 2010-04-29 17:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
playsink: Correctly reconfigure the video chain when switching from a subtitle to a non-subtitle file (1.20 KB, patch)
2010-04-23 14:24 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Julien MOUTTE 2010-04-21 18:47:32 UTC
In playsink reconfigure function there is some code to reenable the video chain if it already exists. In the case of a pipeline with audio, video and subtitles the ghostpad for video is the one from the text chain, for audio video only the ghost pad should be the one from the video chain.

During reconfigure playsink tries to set the ghost correctly depending on the new conditions. I don't understand that block of code though :

      if (!need_vis && !need_text && !playsink->textchain) {
        GST_DEBUG_OBJECT (playsink, "ghosting video sinkpad");
        gst_ghost_pad_set_target (GST_GHOST_PAD_CAST (playsink->video_pad),
            playsink->videochain->sinkpad);
      }

Indeed it does not make sense (IMHO) to check if there's a textchain because if there was one it might be cleaned later on. Can't we just check for need_vis and need_text ?

The consequence of this, as observed on my player, is that playing a A/V file after a A/V/T file is just not working because the ghostpad points to a non existing chain.

The ghostpad could be reset correctly when removing the text chain but just changing the condition to this, makes it work for me :

      if (!need_vis && !need_text) {
        GST_DEBUG_OBJECT (playsink, "ghosting video sinkpad");
        gst_ghost_pad_set_target (GST_GHOST_PAD_CAST (playsink->video_pad),
            playsink->videochain->sinkpad);
      }
Comment 1 Sebastian Dröge (slomo) 2010-04-22 09:13:09 UTC
The reason for this part of code was, that disabling subtitles during playback of the same file should make the video go still through the text chain. This allows seemless re-enabling of the subtitles later without any delays until the first subtitles show up again.
Comment 2 Julien MOUTTE 2010-04-22 11:15:25 UTC
So the correct fix would then be to ghost the video chain on playsink when the textchain is being removed a bit further down in the function right ?
Comment 3 Sebastian Dröge (slomo) 2010-04-22 11:18:43 UTC
Yes, if it is removed further down or if no text chain exists and none will be created.
Comment 4 Sebastian Dröge (slomo) 2010-04-23 14:24:52 UTC
Created attachment 159431 [details] [review]
playsink: Correctly reconfigure the video chain when switching from a subtitle to a non-subtitle file

Fixes bug #616422.
Comment 5 Sebastian Dröge (slomo) 2010-04-23 14:26:06 UTC
(In reply to comment #4)
> Created an attachment (id=159431) [details] [review]
> playsink: Correctly reconfigure the video chain when switching from a subtitle
> to a non-subtitle file
> 
> Fixes bug #616422.

Like this? Does this fix your application?
Comment 6 Sebastian Dröge (slomo) 2010-04-29 17:07:35 UTC
commit 6c9ead703004c4a7a135e027aeba309949d89cb1
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Fri Apr 23 16:24:11 2010 +0200

    playsink: Correctly reconfigure the video chain when switching from a subtit
    
    Fixes bug #616422.