On 12/28/2012 04:15 AM, Fabian Keil wrote: > John Stebbins <[email protected]> wrote: > >> On 11/04/2012 06:33 PM, Fabian Keil wrote: >>> John Stebbins <[email protected]> wrote: >>> >>>> On 11/04/2012 02:51 PM, Fabian Keil wrote: >>>>> John Stebbins <[email protected]> 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
This error is not due to the pgc deduplication. It is caused by an invalid cell_playback_offset value on the disc. libdvdread does not allocate a cell_playback in this case and applications must check that the value is not null. -- John GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01 83F0 49F1 D7B2 60D4 D0F7
signature.asc
Description: OpenPGP digital signature
_______________________________________________ DVDnav-discuss mailing list [email protected] https://lists.mplayerhq.hu/mailman/listinfo/dvdnav-discuss
