[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 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.

I haven't noticed any problems with the patches so far.

I recently had to use another patch (attached) for "Ghost Protocol",
but I haven't verified that the problem actually was due to the pgc
deduplication.

> 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.

The rip was created with vobcopy patched with:
http://www.fabiankeil.de/sourcecode/vobcopy-1.2.0-wip.patch

The command line was:
vobcopy -m -F $BLOCK_COUNT -v -s -S -i /cdrom/
BLOCK_COUNT was most likely 64.

The time stamps seem to imply that the commit bea80e5930 might have been
used, however due to rebasing it likely isn't an exact match and recreating
the problem with this patch set may not be possible.

Fabian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libdvdnav-4.2.0-Let-dvdnav_describe_title_chapters-skip-PGCs-with-mi.diff
Type: text/x-patch
Size: 872 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/attachments/20121228/0f11e561/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/20121228/0f11e561/attachment.asc>