Broken libmmbd with libbdplus

[Hen23776]

Hi there,

There is a bug with libmmbd and some disks using bdplus encoding.

First of all, using MakeMkv directly works without issues.

However, any player (VLC, HandBrake...) using libmmbd (or libbdplus emulation) will start receiving garbage after about 20 minutes. It looks like only the first part of the FUT is being applied even though libmmbd thinks it is using the whole table (nothing on the log)

I have only seen this with James Bond movies (1080 BDs), but it looks like all of them do the same, regardless of the version of BD+ encoding.

I reported the issue earlier this year but I guess it was the wrong forum. Also, it looks like I'm not the only one seeing the issue.

I hope this time Mike sees the post

[pikmin_tom]

Hi,

I've only just started to try using libmmbd for direct disc playback in VLC and I've seen exactly the same behaviour with the video starting to be corrupted after 15 or 20 minutes. I don't think I have many BD+ discs but it happens with both of the discs I've tried: Skyfall and Avatar. I have three BD drives, all different models, and it happens with all of them.

I'm running Linux and I've tried with both the most recent v1.18.3 and older v1.17.7 versions of MakeMKV and it happens with both. If I rip the discs directly with MakeMKV then they rip perfectly.

[MrPenguin]

I've only just started to try using libmmbd for direct disc playback in VLC and I've seen exactly the same behaviour with the video starting to be corrupted after 15 or 20 minutes. I don't think I have many BD+ discs but it happens with both of the discs I've tried: Skyfall and Avatar. I have three BD drives, all different models, and it happens with all of them.

Personally, I don't recommend "direct disc playback" because it puts unnecessary wear-and-tear on your drive. And such drives are no longer being manufactured, so are becoming increasingly difficult to replace.

Suppose you create an encrypted back-up of one of these disks, i.e. replicating the contents of the BD on your hard drive: Can VLC play the back-up cleanly? Because if so, maybe there's an occasional glitch in the data stream from the drive over the data bus (USB?), and VLC isn't coping with it successfully? However, a data stream read from a hard drive should be smoother...

[Sayaka]

Personally, I don't recommend "direct disc playback" because it puts unnecessary wear-and-tear on your drive.

Is this true even if you plan on watching a Blu-ray movie only once ? Could anyone please clarify this ? It would take forever to make a backup copy of each disc from my Walking Dead collection (not to mention I believe it would use up a lot of storage space), and once I'm done watching the seasons I missed I am unlikely to have the time needed to go through the whole series again, so I just had to ask - making a backup of each disc just doesn't seem worth it right now.

[Billycar11]

Personally, I don't recommend "direct disc playback" because it puts unnecessary wear-and-tear on your drive.

Is this true even if you plan on watching a Blu-ray movie only once ? Could anyone please clarify this ? It would take forever to make a backup copy of each disc from my Walking Dead collection (not to mention I believe it would use up a lot of storage space), and once I'm done watching the seasons I missed I am unlikely to have the time needed to go through the whole series again, so I just had to ask - making a backup of each disc just doesn't seem worth it right now.

i backed up the whole walking dead its like 2tb
but i used 6 drives at a time to do it

[pikmin_tom]

Personally, I don't recommend "direct disc playback" because it puts unnecessary wear-and-tear on your drive. And such drives are no longer being manufactured, so are becoming increasingly difficult to replace.

I mostly do rip discs in advance, but sometimes it's convenient to just play them, e.g. if I have a group of friends round and one brings a film to watch, I want to just play it, not wait an hour for it to rip first!

Suppose you create an encrypted back-up of one of these disks, i.e. replicating the contents of the BD on your hard drive: Can VLC play the back-up cleanly? Because if so, maybe there's an occasional glitch in the data stream from the drive over the data bus (USB?), and VLC isn't coping with it successfully? However, a data stream read from a hard drive should be smoother...

I have just tried this and get exactly the same result when played from the hard drive so it's definitely not because the drive can't keep up.

It might be worth noting that this is not specific to VLC as the same happens if I use Kodi, both directly from the disc or hard drive backup. Also if I correctly install libaacs and libbdplus and the necessary files (KEYDB.cfg, VM0 and the conversion tables) so that libmmbd is not used, then the film plays perfectly, so there's no problem with the disc or the drive or the player software, it's just libmmbd.

[rkjnsn]

I ran into this today with MakeMKV 1.18.4 (haven't tried older versions, yet).

After some testing, the issue seems to happen exactly when playback crosses the 4GiB point in a clip. For the first 4GiB of the clip, bdplus_fixup works perfectly. After that, it erroneously wraps and starts applying the fixups from the first 4GiB again. E.g., at 6GiB in, bdplus_fixup is applying the fixups meant for 2GiB in. Obviously, this doesn't work, playback gets messed up.

libmmbd itself looks to be correctly sending all 64 bits of the offset to makemkvcon:

Code: Select all

    m_mem->args[0]=NameFlags;
    m_mem->args[1]=*ClipInfo;
    m_mem->args[2]=(uint32_t)FileOffset;
    m_mem->args[3]=(uint32_t)(FileOffset>>32);

4GiB = 2³² so it seems like the issue is that makemkvcon is ignoring the upper bits in the fourth field and only considering the lower 32-bits of the offset when returning fixups.

[rkjnsn]

Okay, I think I've gotten to the bottom of it. BD+ discs work properly with libmmbd in 1.15.4. That version used shared memory to communicate between libmmbd and makemkvcon. Later versions (checked 1.16.7, 1.17.7, and the current 1.18.4) switched to using pipes for that communication. In the process, they introduced two bugs that impact libbdplus emulation.

Bug 1: High bits of offset lost

This is the bug that causes the garbage after 20 minutes.

Before the change:

Code: Select all

typedef struct _AP_SHMEM
{
    uint32_t    cmd;
    uint8_t     start;
    uint8_t     abort_nomem;
    uint8_t     exit;
    uint8_t     pad0[1];
    uint32_t    pad1[4];
    uint64_t    args[32];          // <-- 64-bit
    utf16_t     strbuf[32768];
} ALIGN_PACKED AP_SHMEM;

Code: Select all

const uint8_t* CMMBDApClient::DecryptUnitMMBD(uint32_t NameFlags,uint32_t* ClipInfo,uint64_t FileOffset,const uint8_t* Data,unsigned int Size)
{
    m_mem->args[0]=NameFlags;
    m_mem->args[1]=*ClipInfo;
    m_mem->args[2]=FileOffset;     // <-- whole 64-bit offset, one slot
    if (Size) memcpy((void*)(m_mem->args+4),Data,Size);

    ExecCmd(apCallDecryptUnitMMBD);

After the change:

Code: Select all

typedef struct _AP_SHMEM
{
    uint32_t    cmd;
    uint32_t    flags;
    uint32_t    pad1;
    uint32_t    pad2;
    uint32_t    args[32];          // <-- narrowed to 32-bit
    uint8_t     strbuf[65008];
} AP_SHMEM;

Code: Select all

const uint8_t* CMMBDApClient::DecryptUnitMMBD(uint32_t NameFlags,uint32_t* ClipInfo,uint64_t FileOffset,const uint8_t* Data,unsigned int Size)
{
    m_mem->args[0]=NameFlags;
    m_mem->args[1]=*ClipInfo;
    m_mem->args[2]=(uint32_t)FileOffset;          // <-- low bits
    m_mem->args[3]=(uint32_t)(FileOffset>>32);    // <-- high bits
    if (Size) memcpy((void*)(m_mem->strbuf),Data,Size);

    ExecCmd(apCallDecryptUnitMMBD,4,Size);

So the client is still sending all 64 bits, but split between two arg slots. It appears, however, that makemkvcon is still only looking at the first offset slot, resulting in the offset effectively getting truncated to 32-bits and thus the video stream to be corrupted past the 4GiB mark.

This is a bug in the binary-only makemkvcon that can only be fixed by the MakeMKV team.

Bug 2: Stale fixups applied to subsequent bytes

If a block doesn't have any fixups to apply, makemkvcon will return a zero-size response and the response buffer will not be overwritten. libmmbd does not currently check the return size, though, so if bdplus_fixup is called on a block with no fixups, but the previous library call was to bdplus_fixup for a block that did have fixups, libmmbd will read the stale buffer and erroneously apply those same fixups to the new block. This will also corrupt the video in cases where the client application is not strictly interleaving some other call between bdplus_fixup invocations.

The fix for this is entirely on the open-source libmmbd side:

Code: Select all

--- a/makemkvgui/inc/lgpl/aproxy.h
+++ b/makemkvgui/inc/lgpl/aproxy.h
@@ -356,7 +356,7 @@
     bool InitMMBD(const char* argp[]);
     bool OpenMMBD(const char* Prefix,const char* Locator);
     const uint8_t* DiscInfoMMBD(uint32_t *Flags,uint8_t *BusKey,uint8_t *DiscId,uint32_t *MkbVersion,uint32_t* ClipCount);
-    const uint8_t* DecryptUnitMMBD(uint32_t NameFlags,uint32_t* ClipInfo,uint64_t FileOffset,const uint8_t* Data,unsigned int Size);
+    const uint8_t* DecryptUnitMMBD(uint32_t NameFlags,uint32_t* ClipInfo,uint64_t FileOffset,const uint8_t* Data,unsigned int Size,unsigned int* RetSize);
 };
 
 class CShMemTransport : public CApClient::ITransport
--- a/libmmbd/src/marmmbd.cpp
+++ b/libmmbd/src/marmmbd.cpp
@@ -99,7 +99,7 @@
     return p+36;
 }
 
-const uint8_t* CMMBDApClient::DecryptUnitMMBD(uint32_t NameFlags,uint32_t* ClipInfo,uint64_t FileOffset,const uint8_t* Data,unsigned int Size)
+const uint8_t* CMMBDApClient::DecryptUnitMMBD(uint32_t NameFlags,uint32_t* ClipInfo,uint64_t FileOffset,const uint8_t* Data,unsigned int Size,unsigned int* RetSize)
 {
     m_mem->args[0]=NameFlags;
     m_mem->args[1]=*ClipInfo;
@@ -113,6 +113,10 @@
 
     *ClipInfo = (uint32_t)m_mem->args[1];
 
+    // CPipeTransport::RecvCmd only refreshes strbuf when the reply carries data, so on
+    // a reply of size zero it still holds whatever the previous reply left there.
+    *RetSize = (unsigned int)(m_mem->cmd&0xffff);
+
     return (const uint8_t*)(m_mem->strbuf);
 }
 
--- a/libmmbd/src/mmconn.cpp
+++ b/libmmbd/src/mmconn.cpp
@@ -380,7 +380,7 @@
     uint32_t clip_info = 0;
     uint32_t *p_clip_info = NULL;
     const uint8_t* dbuf;
-    unsigned int size,ret=0;
+    unsigned int size,ret=0,dbuf_size=0,dbuf_flags;
     uint32_t clip_name = name_flags&(0xfffff|MMBD_FLAG_AUTO_CPSID|MMBD_FILE_SSIF);
 
     if ((!m_active) || (!m_disc_flags)) return -2;
@@ -412,9 +412,14 @@
     }
 
 
-    dbuf = m_apc.DecryptUnitMMBD(name_flags,&clip_info,file_offset,buf,size);
+    dbuf = m_apc.DecryptUnitMMBD(name_flags,&clip_info,file_offset,buf,size,&dbuf_size);
     if (!dbuf) return -4;
 
+    // A reply of size zero leaves the previous reply's bytes in the buffer, so the
+    // buffer's flags are stale and not meant for us.
+    dbuf_flags = dbuf_size ? dbuf[0] : 0;
+
     if (p_clip_info) {
         if (p_clip_info[1]!=clip_info) {
             m_last_clip_info[0]=0xffffffff;
@@ -426,7 +431,7 @@
 
 
     if (0!=(name_flags&MMBD_FLAG_BLOCK_KEY)) {
-        if (dbuf[0]&1) {
+        if (dbuf_flags&1) {
             memcpy(buf, dbuf+1, 16);
         } else {
             memset(buf, 0x00, 16);
@@ -438,7 +443,7 @@
     if ( ((buf[0]&0xC0)!=0x00) && (0==(name_flags&MMBD_FLAG_BDPLUS_ONLY)) ) {
 
 
-        if (dbuf[0]&1) {
+        if (dbuf_flags&1) {
 
             if ((m_disc_flags&AP_MMBD_DISC_FLAG_BUSENC)!=0) {
                 (*m_aes_cbcd)(m_bus_key, iv, buf+16+0*BD_SECTOR_SIZE, BD_SECTOR_SIZE-16);
@@ -456,7 +461,7 @@
     }
 
 
-    if (dbuf[0]&2) {
+    if (dbuf_flags&2) {
         const uint8_t* p = dbuf + 17;
         unsigned int count = *p++;
         for (unsigned int i=0;i<count;i++) {