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 669310 - dvbsuboverlay: some subpictures not rendered (missing end-of-display markers)
dvbsuboverlay: some subpictures not rendered (missing end-of-display markers)
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 0.10.24
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-02-03 13:49 UTC by Jaroslav Klaus
Modified: 2012-07-09 16:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
dvbsuboverlay: do not drop subtitles when no 'end of display' was found (2.03 KB, patch)
2012-02-03 18:48 UTC, Vincent Penquerc'h
none Details | Review

Description Jaroslav Klaus 2012-02-03 13:49:05 UTC
Playbin2 doesn't render subtitles from my DVB transport stream:

% gst-launch playbin2 uri="http://hell.sh.cvut.cz/mg/ch-yes2.ts"

I've tried also to select text (current-text) when streams are detected but still without success.  Although playbin2 creates overlay elements inside. Thx.
Comment 1 Vincent Penquerc'h 2012-02-03 16:11:03 UTC
Confirmed with latest git.
Comment 2 Vincent Penquerc'h 2012-02-03 18:48:17 UTC
Created attachment 206709 [details] [review]
dvbsuboverlay: do not drop subtitles when no 'end of display' was found

The 'end of display' handler code seems to not read the packet at all,
but actually queues the new subtitle onto the overlay. Some sample I've
seen has DVB subtitles that do not have that 'end of display', and they
look believable when the queuing code is called even in the absence of
this part. So it seems safe to call at the end of parsing, if it was
not called yet for this subtitle.

This fixes subtitles in the sample on the bug below.
Comment 3 Jaroslav Klaus 2012-02-03 20:09:44 UTC
Thank you very much. Your patch works.
Comment 4 Tim-Philipp Müller 2012-02-03 23:04:42 UTC
Let's leave this bug open until the fix has actually been committed, so it's not forgotten :)
Comment 5 Vincent Penquerc'h 2012-02-04 00:04:45 UTC
Hah, good point :D
I'd like leio to check it's alright before pushing, to make sure I'm not just papering over the cracks.
Comment 6 Jaroslav Klaus 2012-02-04 07:56:49 UTC
One more thing. When I change text track using PAUSED -> current-text=1 -> PLAYING in glib's timeout subtitles never come back sometimes. It seems to me it happens when I change track during the time some subtitles are just rendered or there are some other activities in overlay.

Here is another a bit longer example http://hell.sh.cvut.cz/mg/ch-yes2-2.ts. When I change track to '1' at 2.8s after start (filesystem access not http) they won't be rendered anymore in 60% of cases at least.

Shall I file it as a different bug? I don't know how much it is related to this one.
Comment 7 Vincent Penquerc'h 2012-02-04 09:31:54 UTC
Probably 600648.

Or possibly 660233.
Comment 8 Mart Raudsepp 2012-02-07 00:45:22 UTC
Quotes from spec:

"display set: set of subtitle segments of a specific subtitle service to which the same PTS value is associated"

"end of display set segment; the end of display set segment contains no internal information, but is used to signal explicitly that no more segments need to be received before the decoding of the current display set can commence."

"The end_of_display_set_segment provides an explicit indication to the decoder that transmission of a display set is complete. The end_of_display_set_segment shall be inserted into the stream immediately after the last object_data_segment for each display set. It shall be present for each subtitle service in a subtitle stream, although decoders need not take advantage of this segment and may apply other strategies to determine when they have sufficient information from a display set to commence decoding."

I think there could be one problem with the patch: I don't think that one GstBuffer that is passed to libdvbsub from gst_dvbsub_overlay_process_text is guaranteed to be the only PES packet for that PTS (but of course usually it is as it seems). Perhaps there's another packet with the same PTS, that continues with more object or other data?
But perhaps the MPEG-TS spec says that it has to be increasing, and can't be the same as continued data? Not sure offhand.
If we don't have a guarantee, at the very least we could send the previous off once a different (presumably increased) PTS hits us. Then we'd have them all also displayed, but the very last would be problematic, or perhaps others too if the following one is in the far future. Perhaps we can do something better after some thinking. Or perhaps it's not a problem at all, we need to check the PES packet related spec sections.

Looking at that, I started to think about the dependencies on how the demuxer gives us subtitle GstBuffers, and it seems that it expects a non-split PES packet, so that is probably fine. However feed_with_pts only consumes one PES packet, so if the demuxer could give multiple in one GstBuffer, we may lose the data from the other ones, as the caller doesn't currently really look at the return value that signifies the position or an error (if position isn't end, there may be more PES packets).

Hope this quick pre-sleep rambling helps
Comment 9 Mart Raudsepp 2012-02-07 12:17:32 UTC
(In reply to comment #6)
> Here is another a bit longer example http://hell.sh.cvut.cz/mg/ch-yes2-2.ts.
> When I change track to '1' at 2.8s after start (filesystem access not http)
> they won't be rendered anymore in 60% of cases at least.
> 
> Shall I file it as a different bug? I don't know how much it is related to this
> one.

The files ch-yes2.ts and ch-yes2-2.ts are currently identical files on your server
Comment 10 Jaroslav Klaus 2012-02-07 13:56:29 UTC
(In reply to comment #9)
> The files ch-yes2.ts and ch-yes2-2.ts are currently identical files on your
> server

Sorry. I've just updated the files so you can test:

http://hell.sh.cvut.cz/mg/ch-yes2.ts
http://hell.sh.cvut.cz/mg/ch-yes2-2.ts
http://hell.sh.cvut.cz/mg/ch-eurosport.ts (HD)
Comment 11 Mark Nauwelaerts 2012-07-09 16:24:46 UTC
Another quote from spec:

"The PTS in the PES packet header provides presentation timing information for the subtitling data, and is associated with the subtitle data in all segments carried in that PES packet. The PTS defines the time at which the associated
decoded segments should be presented. This may include removal of subtitles, for example when an entire region is removed or when all objects in a region are removed. There may be two or more PES packets with the same PTS valuefor example when it is not possible or desirable to include all segments associated to the same PTS in one PES packet. 

The complete set of segments of a subtitle service that are associated to the same PTS is referred to as a display set. The last segment of a display set shall be followed by an "end_of-display-set segment", which signals that no more
subtitling data associated to a certain PTS is needed for that service before decoding can commence. The display sets shall be delivered in their correct presentation-order, and the PTSs of subsequent display sets shall differ by more than one video frame period."

So, as noted above, it is not quite safe to force end-of-display-set upon each (PES) chunk of data delivered to dvb-sub code.

Following commit forces end-of-display-set slightly safer and has subtitles showing up as well:

commit a926d6d1186410ff83e234877cd59af6db6c2613
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Mon Jul 9 18:07:24 2012 +0200

    dvbsuboverlay: properly force end-of-display-set
    
    ... which is upon receiving new data with different PTS spec-wise,
    or optionally upon each packet of subtitle data if desired by property.
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=669310