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

[John Stebbins]

On 12/28/2012 04:15 AM, Fabian Keil wrote:
> 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

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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: OpenPGP digital signature
URL: <http://lists.mplayerhq.hu/pipermail/dvdnav-discuss/attachments/20121228/92643134/attachment.asc>