[DVDnav-discuss] [PATCH] handle duplicate language units and pgc's more gracefully

[John Stebbins]

On 11/04/2012 06:33 PM, Fabian Keil wrote:
> John Stebbins <stebbins at jetheaddev.com> wrote:
>
>> On 11/04/2012 02:51 PM, Fabian Keil wrote:
>>> John Stebbins <stebbins at jetheaddev.com> wrote:
>>>
>>>> This patch never got accepted (probably because it's touches a lot and
>>>> made you nervous), but HandBrake has been applying this for 3 years
>>>> now.
>>>>
>>>> Some discs have large numbers of repeated language units and pgc's
>>>> that cause extreme memory consumption and file i/o issues. scanning
>>>> all the titles on such discs is unusably slow. detect such repeats
>>>> and ref-count the data structures to avoid reading and storing
>>>> duplicate info.
>>> A couple of days ago dvdbackup got killed after the system ran out of
>>> swap space (2 GB), I applied this patch and it solved the problem.
>>> Awesome.
>>>
>>> While I assumed that the changes from malloc() to calloc() might
>>> also make crashes due to bogus pgc field values less likely, the
>>> patch actually causes at least one crash that doesn't happen without
>>> it:
>>>
>>> Program terminated with signal 10, Bus error.
>>> #0  0x0000000802a2390a in dvdnav_describe_title_chapters
>>> (this=0x80cd03e00, title=17, times=0x7fffff8fa298,
>>> duration=0x7fffff8fa2a0)
>>> at /usr/obj-ports/usr/ports/multimedia/libdvdnav/work/libdvdnav-4.2.0/src/searching.c:628
>>> 628	    if(ptt[i].pgn > pgc->nr_of_programs) { (gdb) p *pgc
>>> Cannot access memory at address 0xe000e000000a620
>>>
>>> Everyone's favourite function dvdnav_describe_title_chapters() strikes
>>> again ...
>>>
>>> I haven't tracked down the cause yet.
>>>
>>> Letting the dup_* functions always return -1 doesn't seem to affect the
>>> problem and neither does modifying find_dup_pgc() to compare the whole
>>> pgc instead of just the start_byte or removing the added return in
>>> ifoRead_VTS_PTT_SRPT().
>>>
>>> As far as I can tell the DVD doesn't actually have duplicated pgcs.
>> What disc does it crash on?  HandBrake does not use
>> dvdnav_describe_title_chapters(), so this is a code path we have not
>> tested well enough it sounds like.  dvdbackup uses this?
> dvdbackup doesn't use dvdnav_describe_title_chapters() either.
>
> I failed to mention that the crashing application is vlc.
>
> The DVD is a rip of Monty Python season 3 DVD 1 where unreadable
> sector regions have been padded with surrounding sectors using a
> flawed padding strategy, which probably made the "copy" even less
> standard-compliant than the original.
>
> Given the content, it seems very appropriate that the attached patch,
> which prevents this and other crashes, is somewhat silly ...
>
> I made the skip conditions up while going through backtraces until
> vlc stopped crashing. I haven't properly tested yet how they affect
> other DVDs.
>
>
Thanks. I have the whole Monty Python set.  So when I get home in a
couple of weeks, I'll try to reproduce and see if I can track down the
root cause.  Can you give me more details about how you created the rip
so I can reproduce?  It may be a bad rip, but I don't want to make
dvdnav more fragile, so it's worth looking into.