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 777400 - dvdspu: fix crash when display_rect.bottom isn't even
dvdspu: fix crash when display_rect.bottom isn't even
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.10.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-01-17 15:55 UTC by bob
Modified: 2017-01-19 10:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-dvdspu-fix-crash-when-rendering-subs-w-odd-coords.patch (1.24 KB, patch)
2017-01-17 15:56 UTC, bob
rejected Details | Review
GST_DEBUG=gstspu:6 (184.97 KB, text/plain)
2017-01-17 22:51 UTC, bob
  Details
dvdspu: Handle vob display rect starting on an odd line (3.64 KB, patch)
2017-01-18 00:07 UTC, Jan Schmidt
none Details | Review
dvdspu: Handle vob display rect starting on an odd line (3.77 KB, patch)
2017-01-18 00:15 UTC, Jan Schmidt
none Details | Review
Screenshot of missing scan lines (21.82 KB, image/png)
2017-01-18 00:24 UTC, bob
  Details
Problematic vobsub sample (2.19 MB, application/octet-stream)
2017-01-18 00:37 UTC, bob
  Details
dvdspu: Handle vob display rect starting on an odd line (5.70 KB, patch)
2017-01-18 01:15 UTC, Jan Schmidt
committed Details | Review

Description bob 2017-01-17 15:55:42 UTC
I have a set of sample files whose DVD subtitles are a bit odd. The ffmpeg render just positions them poorly, but GStreamer apps abort() because of a failed assertion in gstspu-vobsub-render.c. Just to see what would happen, I removed the assertion and it appears to work fine.

I haven't the slightest idea why this assertion is there in the first place, so someone who does might have to make sure this patch isn't a regression ;)
Comment 1 bob 2017-01-17 15:56:28 UTC
Created attachment 343662 [details] [review]
0001-dvdspu-fix-crash-when-rendering-subs-w-odd-coords.patch
Comment 2 Jan Schmidt 2017-01-17 22:47:11 UTC
It probably should be an assert, because those are bad - but there's something odd there.

We're supposed to start rendering subs on an even Y coordinate, then render pairs of lines so we can blend the half-sampled chroma easily. At the end is this clause, which may render 1 final line - but that line should always be on an even line, because we started on an even line and rendered in pairs.

The implication is that the top coordinate of these DVD subs is starting on an odd line and we should look at it more closely (also not valid by the DVD spec IIRC).

Can you please provide a GST_DEBUG=gstspu:6 debug log?
Comment 3 bob 2017-01-17 22:51:13 UTC
Created attachment 343679 [details]
GST_DEBUG=gstspu:6

Here you go!
Comment 4 Jan Schmidt 2017-01-17 22:59:52 UTC
Thanks! As suspected, it starts on an odd line:

197:gst_dvd_spu_exec_cmd_blk:<renderer>  Set Display Area top 647 left 0 bottom 695 right 1279
Comment 5 Jan Schmidt 2017-01-18 00:07:40 UTC
Created attachment 343680 [details] [review]
dvdspu: Handle vob display rect starting on an odd line

DVDs always have subpictures that start on an even Y
coordinate, but gstspu does more generic vobsubs these
days, so handle ones that start on an odd vertical position.
Comment 6 Jan Schmidt 2017-01-18 00:08:21 UTC
Please let me know if this patch fixes it and renders ok - or - if you can - please provide a sample.
Comment 7 bob 2017-01-18 00:14:16 UTC
Yep, seems to work fine. Thanks!
Comment 8 Jan Schmidt 2017-01-18 00:15:31 UTC
Created attachment 343681 [details] [review]
dvdspu: Handle vob display rect starting on an odd line

DVDs always have subpictures that start on an even Y
coordinate, but gstspu does more generic vobsubs these
days, so handle ones that start on an odd vertical position.
Comment 9 Jan Schmidt 2017-01-18 00:20:04 UTC
Thanks for testing. I realised I made one small typo in the first patch though - sorry!
Comment 10 bob 2017-01-18 00:22:57 UTC
That one works too, but every other scan line is omitted. Lemme get a screenshot for you
Comment 11 bob 2017-01-18 00:24:33 UTC
Created attachment 343682 [details]
Screenshot of missing scan lines
Comment 12 bob 2017-01-18 00:37:41 UTC
Created attachment 343683 [details]
Problematic vobsub sample

These are the subtitles I've been using as a test case. The second set of dialogue to appear on screen is both what triggered the assert and was rendered strangely.

Extracted with `ffmpeg -i <input>.mkv -an -vn -c:s copy -f rawvideo out`
Comment 13 Jan Schmidt 2017-01-18 00:56:15 UTC
That test sample isn't recognisable, sorry. I can reproduce with a standard file and a small hack to skip the first line of rendering though.
Comment 14 Jan Schmidt 2017-01-18 01:15:53 UTC
Created attachment 343685 [details] [review]
dvdspu: Handle vob display rect starting on an odd line

DVDs always have subpictures that start on an even Y
coordinate, but gstspu does more generic vobsubs these
days, so handle ones that start on an odd vertical position.
Comment 15 Jan Schmidt 2017-01-18 01:17:00 UTC
This patch should fix the rendering.
Comment 16 bob 2017-01-18 01:19:08 UTC
Works great over here
Comment 17 Jan Schmidt 2017-01-18 02:34:17 UTC
Thanks! Pushed:

commit 17430e7b82352f1dc8caedddbf47d6c579c8235e
Author: Jan Schmidt <jan@centricular.com>
Date:   Wed Jan 18 11:05:21 2017 +1100

    dvdspu: Handle vob display rect starting on an odd line
    
    DVDs always have subpictures that start on an even Y
    coordinate, but gstspu does more generic vobsubs these
    days, so handle ones that start on an odd vertical position.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=777400
Comment 18 Sebastian Dröge (slomo) 2017-01-18 09:51:23 UTC
Should this go into 1.10?
Comment 19 Jan Schmidt 2017-01-18 11:50:18 UTC
(In reply to Sebastian Dröge (slomo) from comment #18)
> Should this go into 1.10?

Yes