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

[Fabian Keil]

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.

Fabian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libdvdnav-4.2.0-Add-a-couple-of-additional-sanity-checks-for-dvdnav_.patch
Type: text/x-patch
Size: 1358 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/attachments/20121104/aba31afa/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/attachments/20121104/aba31afa/attachment.asc>