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 740565 - dvdsub to kate subtitle transcoding fails after first subtitle for ogg container
dvdsub to kate subtitle transcoding fails after first subtitle for ogg container
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.2.4
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-23 01:13 UTC by Eric
Modified: 2018-11-03 11:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
debug output for failing pipeline (1.09 MB, application/gzip)
2014-11-26 18:41 UTC, Eric
  Details
collectpads fix for sparse pads (9.62 KB, patch)
2014-12-17 14:17 UTC, Vincent Penquerc'h
rejected Details | Review
oggmux fix for deadlock popping buffers from collectpads (2.12 KB, patch)
2014-12-17 14:18 UTC, Vincent Penquerc'h
none Details | Review
fix deadlock when not pulling a buffer from collectpads - updated (2.12 KB, patch)
2015-01-25 09:55 UTC, Vincent Penquerc'h
rejected Details | Review

Description Eric 2014-11-23 01:13:40 UTC
This command goes on showing output for all subtitle entries:
gst-launch-1.0 filesrc location=Arigatou.Death.Note.Ep01.\[x264.AAC\]\[558F2901\].mkv ! matroskademux ! dvdsubparse ! kateenc category=spu-subtitles ! fakesink silent=false -v

While this one stops immediately:
gst-launch-1.0 filesrc location=Arigatou.Death.Note.Ep01.\[x264.AAC\]\[558F2901\].mkv ! matroskademux ! dvdsubparse ! kateenc category=spu-subtitles ! oggmux ! fakesink silent=false -v

I can sink the former one into a file, but ogg tools can't merge it with the video. I think because it's not an ogg container itself. I'm guessing it's just a raw byte stream of kate?

When I sink the latter to a file and merge it with video, I get the very first line of subtitle and then nothing else.

Running version: 1.2.4-1~ubuntu1:amd64 on Linux Mint
Comment 1 Thiago Sousa Santos 2014-11-23 07:34:21 UTC
Does it show any error in the output? Can you share a file that reproduces the issue for testing?

Please attach a GST_DEBUG=6 log for investigation.
Comment 2 Eric 2014-11-23 20:36:17 UTC
No errors that I can see. Where do I set GST_DEBUG? Is that just an environment variable?

Here's the pastbin for the fakesink that stops:
http://pastebin.com/yKanPKSY

Here's a link to the file I'm trying to transcode the subs from:
https://www.sendspace.com/file/oz3kq6
Comment 3 Thiago Sousa Santos 2014-11-23 21:01:28 UTC
GST_DEBUG is an environment variable to trigger gst debug messages.


Should work with any gstreamer application as long as your gstreamer has debug logs compiled into it.

GST_DEBUG=6 gst-launch-1.0 <your pipeline> > gst.log 2>&1

compress that gst.log file and attach it here.
Comment 4 Luis de Bethencourt 2014-11-23 21:47:41 UTC
In case you are curious, I think this explains it really well :)

http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/gst-running.html

Thanks for your bug report!
Comment 5 Eric 2014-11-26 18:41:34 UTC
Created attachment 291578 [details]
debug output for failing pipeline

Attaching the output of:
GST_DEBUG=6 gst-launch-1.0 filesrc location=Arigatou.Death.Note.Ep01.\[x264.AAC\]\[558F2901\].mkv ! matroskademux ! dvdsubparse ! kateenc category=spu-subtitles ! oggmux ! filesink location=test.ogg > gst.log 2>&1
Comment 6 Vincent Penquerc'h 2014-12-17 14:16:16 UTC
Pretty intricate issue in collectpads, it seems.

I think the semantics of "queued" in collectpads were inconsistent. The attached patches fix the bug on the supplied command line and test file.

It would be good if someone more familiar with collectpads could double check the patch.

Additionally, a patch to oggmux was needed to ensure "free" pad buffer is replaced from collectpads, to avoid collectpads deadlocking, waiting for the colletpads buffer to be moved.

Such a change to collectpads feels rather invasive, so may have side effects in other places. Passes core and -base tests, at least.
Comment 7 Vincent Penquerc'h 2014-12-17 14:17:09 UTC
Created attachment 292903 [details] [review]
collectpads fix for sparse pads
Comment 8 Vincent Penquerc'h 2014-12-17 14:18:01 UTC
Created attachment 292904 [details] [review]
oggmux fix for deadlock popping buffers from collectpads
Comment 9 Eric 2015-01-25 07:19:39 UTC
These patches don't seem to apply to existent files in the source tree. What am I missing?
Comment 10 Vincent Penquerc'h 2015-01-25 09:55:41 UTC
Created attachment 295364 [details] [review]
fix deadlock when not pulling a buffer from collectpads - updated

-base patch updated to latest master. The core patch still applies fine.
Comment 11 Vincent Penquerc'h 2015-01-25 09:57:39 UTC
Ah - this was filed against -bad, but the issue actually was in core/-base, so you need to apply the patches to those trees instead of -bad. I'll change the component to -base, since that's where the main bug is.
Comment 12 Eric 2015-01-25 17:23:21 UTC
I see now. One patch is against the gst-plugins-base tree and the other is for the core gstreamer tree. Are both these patches for the master branch or were the originals for release 1.2.4?
Comment 13 Vincent Penquerc'h 2015-01-25 22:22:15 UTC
All for master, though I would expect they should apply to 1.2.4 without much trouble.
Comment 14 Eric 2015-01-25 23:32:20 UTC
Installed on Gentoo using user patches. The first patches apply cleanly with release 1.4.5, and the problem is fixed. The patches fail to apply to 1.2.4 as is.

I'm guessing we wait for someone else to mark this resolved when they commit these changes to the repository?
Comment 15 Vincent Penquerc'h 2015-01-26 11:31:34 UTC
That's the idea, but the change to collectpads is fairly intrusive (and so possible source of regressions), and collectpads is apparently deprecated now, so I'm not sure what the maintainers think of pushing this. I'd like it to get a review first in any case.
Comment 16 Vincent Penquerc'h 2015-02-19 13:51:07 UTC
Any gst maintainer with an opinion on whether this is fine to push, given the intrusiveness of the collectpads change ?
Comment 17 Sebastian Dröge (slomo) 2015-02-19 13:53:01 UTC
I'd prefer to just get it right with aggregator and stop touching collectpads.
Comment 18 Tim-Philipp Müller 2015-02-19 13:56:36 UTC
Yes, let's not do major changes to collectpads at this point.
Comment 19 Vincent Penquerc'h 2015-02-19 14:02:41 UTC
Comment on attachment 292903 [details] [review]
collectpads fix for sparse pads

It's dead, Jim.
Comment 20 Eric 2015-03-15 05:04:56 UTC
Will encoding to kate be supported in future releases then? Or does the kate plugin need rewritten to match API changes or something?
Comment 21 Vincent Penquerc'h 2015-03-16 12:44:10 UTC
It's oggmux that will be changed to use the new aggregator code, instead of the collectpads code.

Anyway, if we apply just the oggmux patch, this fixes the issue for the main case of encoding subtitles along with another (non sparse) stream, such as Theora or Vorbis. The collectpads patch is only needed to fix the case of muxing just kate streams only.

So after looking again at this, I'm tempted to push the oggmux patch alone, assuming maintainers have no objection.
Comment 22 Vincent Penquerc'h 2015-04-03 15:02:23 UTC
The oggmux patch is now up (the collectpads one will not be).
Transcoding of the file should now work, though transcoding only subtitles still fail.


commit b590a843f4b2d452ad304acdcb3265a97181fe7b
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Wed Dec 17 12:17:09 2014 +0000

    oggmux: fix deadlock when not pulling a buffer from collectpads
    
    oggmux keeps a cached buffer per pad, and pulls buffers from
    collectpads to this cached buffer for all pads before processing
    the best pad. In some cases, the move from collectpads buffer
    to cached buffer is delayed till next call. However, when there
    is only one pad, this can't be delayed till next call as there
    will be a deadlock since collectpads has no other pad to push to.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=740565
Comment 23 GStreamer system administrator 2018-11-03 11:33:08 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/146.