embyforkodi (next-gen) 10.X.X support

[seashell 35]

2 minutes ago, quickmic said:

Cached images are downloaded to Kodi (userdata/Thumbnails). http/urls queries are no longer used at this point.

Actually Kodi always caches images once loaded by a view (visible). The cache option does the same but without browsing views. It loads the images in advance.

So that points to somehow skins are getting a bad link to non-cached images.  Could be the skins, but odd that it's two different ones.

[quickmic 1352]

4 minutes ago, seashell said:

So that points to somehow skins are getting a bad link to non-cached images.  Could be the skins, but odd that it's two different ones.

Well, I use estuarymodv2 and the fanarts are loaded without caching. They are lopping as background.

[seashell 35]

1 minute ago, quickmic said:

Well, I use estuarymodv2 and the fanarts are loaded without caching. They are lopping as background.

Ok.  Could be the skins.   Not a big deal.

[seashell 35]

5 hours ago, seashell said:

Ok.  Could be the skins.   Not a big deal.

I'm not sure what's going on.  The same skins can load the fan art on my PC that fail on the tv box, but it's still showing failed attempts to get the images as directories.

Also just saw this one pop up.   Does the Delayed content say anything about this possibly being a timeout somewhere?

 

2024-04-25 13:48:04.059 T:21777   error <general>: EMBY.hooks.webservice: Delayed content not found
2024-04-25 13:48:04.060 T:21681   error <general>: CCurlFile::Open - <http://127.0.0.1:57342/picture/8f96e117caea4dadacbfd99c712ad540/p-122167-0-B-fde6964451d6f1e9d140e03e42694f94/> Failed with code 404:
 

ETA:

Almost certain this is a symptom of a timeout from the delayed content system.  I augmented the "Delayed content not found" message to list the actual content not found and it matches the CCurl and directory lines except for the trailing slash. 

The trailing slash in the Ccurl error log entry may just be a bug in the error message adding an extra slash, and perhaps there's some logic in Kodi that says if an image link fails try it again as a directory?  Or something along those lines but I'm guessing on both of these as I haven't looked at any Kodi code.

Regardless of Kodi's reaction to the delayed content system failing though, I think the underlying issue is something in the timing changed from 20.5 to 21.0 such that the delayed content work around isn't working all that well anymore and it's failing occasionally on my PC and almost always on my TV box.

Edited April 25 by seashell

[seashell 35]

So the issue seems to be a race condition in the DelayedContent handling.  I'm not sure if Kodi 21 is multi-threaded in some area involving image fetching that Kodi 20.5 was not.

What I'm seeing is that in webservice.py around line 512 (sorry, I've got some edits so it likely isn't the exact line number) where it starts the loading process for the artwork into the cache a second call to this function is being made before the first call to the function is complete.   So globals()['DelayedContent'][QueryData['DelayedContentId']] = "" is performed again possibly wiping out the just loaded data and a second load operation is performed.

Here's the lines around 512 I mean.

    if QueryData['Type'] == 'picture':
        if Payload not in ArtworkCache[1]:

Then regardless of how that collision works out it sets up two parallel calls to send_delayed_content.  The first one will be happy and will call client.send(DelayedContent[DelayedContentId]) and then del globals()["DelayedContent"][DelayedContentId] so when the second call comes along the key is gone and the else clause is triggered so

        xbmc.log(f"EMBY.hooks.webservice: Delayed content not found {DelayedContentId}", 3) # LOGERROR
        client.send(sendNoContent)

gets executed which makes Kodi sad.

Of course sometimes they will be close enough that both calls can return the data before the first one does the delete.

 

So why is Kodi loading the same image twice so close together?  No idea.  But it is.  And since the DelayContent system is stateful it needs to be thread safe.

I did a quick kludge by making DelayedContent Entries [whatever_they_were_before,1] on creation and then when the second call comes along it updates the counter.  Then when the del call comes it checks if the count is 1 and deletes it if it is or decrements the counter if not.  With that I've not seen the content not found errors or the odd Kodi failed directory loads on my PC.  Have not tried on the TV box.

There are other places DelayedContent is manipulated in that file though, and I wasn't super careful to trace it all or see if any other files interact with it.

 

Since this code is called threaded the global variables should be protected with mutexes as well.

Edited April 26 by seashell

[quickmic 1352]

4 hours ago, seashell said:

So the issue seems to be a race condition in the DelayedContent handling.  I'm not sure if Kodi 21 is multi-threaded in some area involving image fetching that Kodi 20.5 was not.

What I'm seeing is that in webservice.py around line 512 (sorry, I've got some edits so it likely isn't the exact line number) where it starts the loading process for the artwork into the cache a second call to this function is being made before the first call to the function is complete.   So globals()['DelayedContent'][QueryData['DelayedContentId']] = "" is performed again possibly wiping out the just loaded data and a second load operation is performed.

Here's the lines around 512 I mean.

    if QueryData['Type'] == 'picture':
        if Payload not in ArtworkCache[1]:

Then regardless of how that collision works out it sets up two parallel calls to send_delayed_content.  The first one will be happy and will call client.send(DelayedContent[DelayedContentId]) and then del globals()["DelayedContent"][DelayedContentId] so when the second call comes along the key is gone and the else clause is triggered so

        xbmc.log(f"EMBY.hooks.webservice: Delayed content not found {DelayedContentId}", 3) # LOGERROR
        client.send(sendNoContent)

gets executed which makes Kodi sad.

Of course sometimes they will be close enough that both calls can return the data before the first one does the delete.

 

So why is Kodi loading the same image twice so close together?  No idea.  But it is.  And since the DelayContent system is stateful it needs to be thread safe.

I did a quick kludge by making DelayedContent Entries [whatever_they_were_before,1] on creation and then when the second call comes along it updates the counter.  Then when the del call comes it checks if the count is 1 and deletes it if it is or decrements the counter if not.  With that I've not seen the content not found errors or the odd Kodi failed directory loads on my PC.  Have not tried on the TV box.

There are other places DelayedContent is manipulated in that file though, and I wasn't super careful to trace it all or see if any other files interact with it.

 

Since this code is called threaded the global variables should be protected with mutexes as well.

Before I review this issue, some background info. All this delayedcontent thingies are actually workarounds.

Kodi has a timeout setting in advancedsetting.xml -> curlclienttimeout. This setting is adjustable in plugin settings.

Problem 1: In waiting (for response) state, Kodi is blocking (at least for HEAD requests). Means the complete plugin code is stalled. No notification, no api calls nothing is processed.

Problem 2: HEAD requests are completely blocking. Not even xbmc.api's are accepted/processed while Kodi's curl waiting for HEAD request response.

Ergo, the timeout setting must be quite low, otherwise the plugin (and Kodi) can do nothing at this point until some response is received by Kodi (actually Kodi's curl). There was an issue reported that Kodi freezes on playback stop for e.g. 120 seconds when I intentionally set the timeouts to 120 seconds.

Kodi called an URL (could be a Emby server query, could be something else not responsive). Whatever was called, it was not responding.

So I switched to a very low timeout setting, but there is another issue. The plugin uses a selection messagebox for e.g. multicontent-selection or transcoding-stream-selection waiting for user input. This message box comes up on GET requests (not for HEAD due as described, HEAD are complete blocking) and is processed in the internal webserver.

Obviously 2 seconds are not enough for the user selection, so I need to send Kodi something back lower than the curlclienttimeout settings not running into timeouts.

The only option I found was a redirect to the same (internal webserver) URL but with the prefix "delayed_content". -> "http://127.0.0.1:57342/delayed_content/blabla"

A loop if you want. Each loop checks if the user input was done and finally send the actual redirect to Emby server's stream URL but there was another issue.

The redirects are limited to a certain number, I think it was 10 or 50 (default settings).

Therefore I use the Kodi's url parameter extension "redirect-limit=1000" for Urls e.g. http://127.0.0.1:57342/picture/2a38697ffc1b428b943aa1b6014e2263/p-131797-0-p-95d8e34261a7fa0548e08ba545ddf287|redirect-limit=1000

This increases the redirect limit.

For images (even if there is no user input), 2 seconds could be too low. Depends on Emby server responsiveness and Emby server isn't the fastest at some points. Especially when Emby server is busy with scanning etc. Also images could include overlays which the plugin injects. e.g. for bookmark images. This takes also time to process.

btw, you may noticed, if a user cancelled a selection (for multicontent), the internal/plugins webserver cannot send a 404 to Kodi. This would result in a Kodi message -> content not found.

Therefore I send back a "fake content" ->"BlankWAV" in the code and it's exactly that. A blank wav audio file, fully valid but 0 seconds. Kodi is happy, that it got content, even it's just a dummy and no Kodi message pops up.

 

sideinfo

executebuiltin has also an option to wait for processed. This also stalls all plugins. If the executebuiltin triggers a plugin command, Kodi will freeze as the plugin cannot respond.

All that, I learned the hard way and is not nice. I would call it Kodi bugs.

 

 

 

 

Edited April 26 by quickmic

[quickmic 1352]

3 hours ago, seashell said:

Then regardless of how that collision works out it sets up two parallel calls to send_delayed_content.  The first one will be happy and will call client.send(DelayedContent[DelayedContentId]) and then del globals()["DelayedContent"][DelayedContentId] so when the second call comes along the key is gone and the else clause is triggered so

I'll review that, you might be right. If Kodi requests the same content (nearly simultaneous) twice (which it shouldn't, makes no sense) plugin would wipe the first or second request from its DelayedContent cache. In the past I played a bit using thread ids but according to my research there was an issue. As mentioned before, I send "redirects" and followup request hasn't the same thread id. I could think of a counter as id...

I need to think about it.

 

Keeping the globals()["DelayedContent"] would solve this issue I guess, but it would fill up the memory. Ergo not an option, but as a prove of concept you could remove it.

remove -> "del globals()["DelayedContent"][DelayedContentId]" from the code

Edited April 26 by quickmic

[seashell 35]

Sounds like a mess.  I see now it's your code doing the threading.  From the sounds of the lock ups it's Kodi that should be doing the threading.  Always sad when a UI locks up due to background processes.

56 minutes ago, quickmic said:

I'll review that, you might be right. If Kodi requests the same content (nearly simultaneous) twice (which it shouldn't, makes no sense) plugin would wipe the first or second request from its DelayedContent cache. In the past I played a bit using thread ids but according to my research there was an issue. As mentioned before, I send "redirects" and followup request hasn't the same thread id. I could think of a counter as id...

Keeping the globals()["DelayedContent"] would solve this issue, but it would fill up the memory. Ergo not an option, but as a prove of concept you could remove it.

I need to think about it.

A simple counter as I did should be fine, and has very little extra memory growth.  But the accesses to and logic around the globals should be in critical sections so that only one thread can be manipulating them at any given time, or so that any reaction is still appropriate.  This is true regardless of how you solve the immediate problem being discussed.  If you've done a lot of threading then I'm sure you know what i mean, but if not I can explain more or take a closer look at the code and make some mods.

[quickmic 1352]

8 minutes ago, seashell said:

 But the accesses to and logic around the globals should be in critical sections so that only one thread can be manipulating them at any given time

But it isn't the same thread. The "http://127.0.0.1:57342/delayed_content/blabla" redirects are all running as different threads, therefore it must be a global cache.

Edited April 26 by quickmic

[seashell 35]

10 minutes ago, quickmic said:

But it isn't the same thread. The "http://127.0.0.1:57342/delayed_content/blabla" are all running as different threads, therefore it must be a global cache.

Yes exactly.  You already have a global that is shared amongst all threads.  And when you have that you must protect that only one thread access it a time.  It's what critical sections and mutexes are for.   The key phrase is "at any given time".  It's a lock.  A thread wants to do something so it tries to acquire the lock.  If no thread is using it it gets the lock and does its thing.  If another thread already has the lock the thread is blocked until the other thread is done.  So critical sections must be short.

So for example.

Acquire lock:

Check if global id already exists. 

                 If yes:

                       increment counter

                       release lock

                      return

                if no:

                      create id and set counter to 1

                      release lock

                      actually load the image from emby

 

Then when it's loaded and you're going to put it in global you grab the lock again just long enough to put it in the global dictionary and then release it.

This is the standard way threaded programming works.

Edited April 26 by seashell

[seashell 35]

Here's a log example showing kodi does make the same request twice quickly.  These are hasty print statements I threw in

First request

2024-04-25 17:45:38.518 T:7718     info <general>: EMBY.hooks.webservice: Load artwork data into cache: /picture/8f96e117caea4dadacbfd99c712ad540/p-93861-0-B-3c409f0bdb2ba0c6302f25326f43dcb5
2024-04-25 17:45:38.518 T:7718     info <general>: EMBY.hooks.webservice: Starting Delayed Content Load For /picture/8f96e117caea4dadacbfd99c712ad540/p-93861-0-B-3c409f0bdb2ba0c6302f25326f43dcb5

Second request

2024-04-25 17:45:38.521 T:7719     info <general>: EMBY.hooks.webservice: Load artwork data into cache: /picture/8f96e117caea4dadacbfd99c712ad540/p-93861-0-B-3c409f0bdb2ba0c6302f25326f43dcb5

I intercept it here so it doesn't load it a second time from emby

2024-04-25 17:45:38.521 T:7719     info <general>: ******************************* RACE COND 1 DELAY HAS /picture/8f96e117caea4dadacbfd99c712ad540/p-93861-0-B-3c409f0bdb2ba0c6302f25326f43dcb5

 

blah blah blah lots of checking for delayed content to be loaded...

 

End of first request when the content was actually loaded
2024-04-25 17:45:38.741 T:7718     info <general>: Loaded Delayed Content for /picture/8f96e117caea4dadacbfd99c712ad540/p-93861-0-B-3c409f0bdb2ba0c6302f25326f43dcb5

Edited April 26 by seashell

[quickmic 1352]

7 minutes ago, seashell said:

This is the standard way threaded programming works.

I know what you mean

Quote

First request

2024-04-25 17:45:38.518 T:7718     info <general>: EMBY.hooks.webservice: Load artwork data into cache: /picture/8f96e117caea4dadacbfd99c712ad540/p-93861-0-B-3c409f0bdb2ba0c6302f25326f43dcb5

Second request
2024-04-25 17:45:38.521 T:7719     info <general>: EMBY.hooks.webservice: Load artwork data into cache: /picture/8f96e117caea4dadacbfd99c712ad540/p-93861-0-B-3c409f0bdb2ba0c6302f25326f43dcb5

Did you check it fore the same query type -> both GETs or both POSTs

I assume yes, but just checking...

Edited April 26 by quickmic

[quickmic 1352]

1 minute ago, quickmic said:

I know what you mean

Did you check it fore the same query type -> both GETs or both POSTs

I assume yes, but just checking...

Corection

GET/HEAD I meant

[seashell 35]

1 minute ago, quickmic said:

Did you check it fore the same query type -> both GETs or both POSTs

I assume yes, but just checking...

I did not, because the print statements are in the function that loads the artwork from emby.  So all that mattered to me was the logic was being called twice overlapping.  How that logic got called didn't matter to me, but it could be part of the double load problem.

[seashell 35]

Here's my totally hacked up code that showed those print statements.  I edited out some of the not so important ones.

 

    if QueryData['Type'] == 'picture':
        if Payload not in ArtworkCache[1]:
            xbmc.log(f"EMBY.hooks.webservice: Load artwork data into cache: {Payload}", 1) # LOGDEBUG
            if QueryData['DelayedContentId'] in globals()['DelayedContent']:
                globals()['DelayedContent'][QueryData['DelayedContentId']][1] += 1
                xbmc.log(f"******************************* RACE COND 1 DELAY HAS {Payload}", 1) # LOGDEBUG
                client.send(f"HTTP/1.1 307 Temporary Redirect\r\nServer: Emby-Next-Gen\r\nConnection: close\r\nLocation: http://127.0.0.1:57342/delayed_content/{QueryData['DelayedContentId']}\r\nContent-length: 0\r\n\r\n".encode())
                client.close()
                client = None
                return
            globals()['DelayedContent'][QueryData['DelayedContentId']] = ["",1]
            xbmc.log(f"EMBY.hooks.webservice: Starting Delayed Content Load For {Payload}", 1) # DELETEME
            client.send(f"HTTP/1.1 307 Temporary Redirect\r\nServer: Emby-Next-Gen\r\nConnection: close\r\nLocation: http://127.0.0.1:57342/delayed_content/{QueryData['DelayedContentId']}\r\nContent-length: 0\r\n\r\n".encode())
            client.close()
            client = None
            xbmc.log(f"EMBY.hooks.webservice: Load artwork data: {Payload}", 0) # LOGDEBUG

            # Remove items from artwork cache if mem is over 100MB
            if ArtworkCache[0] > 100000000:
                for PayloadId, ArtworkCacheData in list(ArtworkCache[1].items()):
                    globals()['ArtworkCache'][0] -= ArtworkCacheData[2]
                    del globals()['ArtworkCache'][1][PayloadId]

                    if ArtworkCache[0] < 100000000:
                        break

            if not QueryData['Overlay']:
                BinaryData, ContentType, _ = utils.EmbyServers[QueryData['ServerId']].API.get_Image_Binary(QueryData['EmbyID'], QueryData['ImageType'], QueryData['ImageIndex'], QueryData['ImageTag'])
            else:
                BinaryData, ContentType = utils.image_overlay(QueryData['ImageTag'], QueryData['ServerId'], QueryData['EmbyID'], QueryData['ImageType'], QueryData['ImageIndex'], QueryData['Overlay'])

            ContentSize = len(BinaryData)
            globals()["ArtworkCache"][0] += ContentSize
            globals()["ArtworkCache"][1][Payload] = (f"HTTP/1.1 200 OK\r\nServer: Emby-Next-Gen\r\nConnection: close\r\nContent-Length: {ContentSize}\r\nContent-Type: {ContentType}\r\n\r\n".encode(), BinaryData, ContentSize)
            del BinaryData
            globals()['DelayedContent'][QueryData['DelayedContentId']][0] = ArtworkCache[1][Payload][0] + ArtworkCache[1][Payload][1]
            xbmc.log(f"Loaded Delayed Content for {Payload}", 1) # LOGDEBUG
        else:
            xbmc.log(f"EMBY.hooks.webservice: Load artwork data from cache: {Payload}", 1) # LOGDEBUG
            client.send(ArtworkCache[1][Payload][0] + ArtworkCache[1][Payload][1])

        return

[seashell 35]

Here's the same code with mutexs which in python are threading.Lock objects.   Note I didn't actually run this, so it's a hopefully working example.

 

At the top where you define ArtworkCache and DelayedContent globals also create your locks.

 

    ArtworkCacheLock = threading.Lock()
    DelayedContentLock = threading.Lock()

 

Then in the function

    if QueryData['Type'] == 'picture':
        ArtworkCacheLock.acquire()
        if Payload not in ArtworkCache[1]:
            ArtworkCacheLock.release()
            xbmc.log(f"EMBY.hooks.webservice: Load artwork data into cache: {Payload}", 1) # LOGDEBUG
            DelayedContentLock.acquire()
            if QueryData['DelayedContentId'] in globals()['DelayedContent']:
                globals()['DelayedContent'][QueryData['DelayedContentId']][1] += 1
                DelayedContentLock.release()
                xbmc.log(f"******************************* RACE COND 1 DELAY HAS {Payload}", 1) # LOGDEBUG
                client.send(f"HTTP/1.1 307 Temporary Redirect\r\nServer: Emby-Next-Gen\r\nConnection: close\r\nLocation: http://127.0.0.1:57342/delayed_content/{QueryData['DelayedContentId']}\r\nContent-length: 0\r\n\r\n".encode())
                client.close()
                client = None
                return
            globals()['DelayedContent'][QueryData['DelayedContentId']] = ["",1]
            DelayedContentLock.release()
            xbmc.log(f"EMBY.hooks.webservice: Starting Delayed Content Load For {Payload}", 1) # DELETEME
            client.send(f"HTTP/1.1 307 Temporary Redirect\r\nServer: Emby-Next-Gen\r\nConnection: close\r\nLocation: http://127.0.0.1:57342/delayed_content/{QueryData['DelayedContentId']}\r\nContent-length: 0\r\n\r\n".encode())
            client.close()
            client = None
            xbmc.log(f"EMBY.hooks.webservice: Load artwork data: {Payload}", 0) # LOGDEBUG

            # Remove items from artwork cache if mem is over 100MB
            ArtworkCacheLock.acquire()
            if ArtworkCache[0] > 100000000:
                for PayloadId, ArtworkCacheData in list(ArtworkCache[1].items()):
                    globals()['ArtworkCache'][0] -= ArtworkCacheData[2]
                    del globals()['ArtworkCache'][1][PayloadId]

                    if ArtworkCache[0] < 100000000:
                        break
            ArtworkCacheLock.release()

            if not QueryData['Overlay']:
                BinaryData, ContentType, _ = utils.EmbyServers[QueryData['ServerId']].API.get_Image_Binary(QueryData['EmbyID'], QueryData['ImageType'], QueryData['ImageIndex'], QueryData['ImageTag'])
            else:
                BinaryData, ContentType = utils.image_overlay(QueryData['ImageTag'], QueryData['ServerId'], QueryData['EmbyID'], QueryData['ImageType'], QueryData['ImageIndex'], QueryData['Overlay'])

            ContentSize = len(BinaryData)
            ArtworkCacheLock.acquire()
            globals()["ArtworkCache"][0] += ContentSize
            globals()["ArtworkCache"][1][Payload] = (f"HTTP/1.1 200 OK\r\nServer: Emby-Next-Gen\r\nConnection: close\r\nContent-Length: {ContentSize}\r\nContent-Type: {ContentType}\r\n\r\n".encode(), BinaryData, ContentSize)
            ArtworkCacheLock.release()
            del BinaryData
            DelayedContentLock.acquire()
            globals()['DelayedContent'][QueryData['DelayedContentId']][0] = ArtworkCache[1][Payload][0] + ArtworkCache[1][Payload][1]
            DelayedContentLock.release()
            xbmc.log(f"Loaded Delayed Content for {Payload}", 1) # LOGDEBUG
        else:
            # We still have the lock from above, get the data before another thread possibly overwrites it in the cache for over 100MB
            # but make a copy so we don't hold the lock during the full send
            toSend = ArtworkCache[1][Payload][0] + ArtworkCache[1][Payload][1]
            ArtworkCacheLock.release()
            xbmc.log(f"EMBY.hooks.webservice: Load artwork data from cache: {Payload}", 1) # LOGDEBUG
            client.send(toSend)

        return

Edited April 26 by seashell

[quickmic 1352]

1 minute ago, seashell said:

Here's the same code with mutexs which in python are threading.Lock objects.   Note I didn't actually run this, so it's a hopefully working example.

 

At the top where you define ArtworkCache and DelayedContent globals also create your locks.

 

    ArtworkCacheLock = threading.Lock()
    DelayedContentLock = threading.Lock()

 

Then in the function

    if QueryData['Type'] == 'picture':
        ArtworkCacheLock.acquire()
        if Payload not in ArtworkCache[1]:
            ArtworkCacheLock.release()
            xbmc.log(f"EMBY.hooks.webservice: Load artwork data into cache: {Payload}", 1) # LOGDEBUG
            DelayedContentLock.acquire()
            if QueryData['DelayedContentId'] in globals()['DelayedContent']:
                globals()['DelayedContent'][QueryData['DelayedContentId']][1] += 1
                DelayedContentLock.release()
                xbmc.log(f"******************************* RACE COND 1 DELAY HAS {Payload}", 1) # LOGDEBUG
                client.send(f"HTTP/1.1 307 Temporary Redirect\r\nServer: Emby-Next-Gen\r\nConnection: close\r\nLocation: http://127.0.0.1:57342/delayed_content/{QueryData['DelayedContentId']}\r\nContent-length: 0\r\n\r\n".encode())
                client.close()
                client = None
                return
            globals()['DelayedContent'][QueryData['DelayedContentId']] = ["",1]
            DelayedContentLock.release()
            xbmc.log(f"EMBY.hooks.webservice: Starting Delayed Content Load For {Payload}", 1) # DELETEME
            client.send(f"HTTP/1.1 307 Temporary Redirect\r\nServer: Emby-Next-Gen\r\nConnection: close\r\nLocation: http://127.0.0.1:57342/delayed_content/{QueryData['DelayedContentId']}\r\nContent-length: 0\r\n\r\n".encode())
            client.close()
            client = None
            xbmc.log(f"EMBY.hooks.webservice: Load artwork data: {Payload}", 0) # LOGDEBUG

            # Remove items from artwork cache if mem is over 100MB
            ArtworkCacheLock.acquire()
            if ArtworkCache[0] > 100000000:
                for PayloadId, ArtworkCacheData in list(ArtworkCache[1].items()):
                    globals()['ArtworkCache'][0] -= ArtworkCacheData[2]
                    del globals()['ArtworkCache'][1][PayloadId]

                    if ArtworkCache[0] < 100000000:
                        break
            ArtworkCacheLock.release()

            if not QueryData['Overlay']:
                BinaryData, ContentType, _ = utils.EmbyServers[QueryData['ServerId']].API.get_Image_Binary(QueryData['EmbyID'], QueryData['ImageType'], QueryData['ImageIndex'], QueryData['ImageTag'])
            else:
                BinaryData, ContentType = utils.image_overlay(QueryData['ImageTag'], QueryData['ServerId'], QueryData['EmbyID'], QueryData['ImageType'], QueryData['ImageIndex'], QueryData['Overlay'])

            ContentSize = len(BinaryData)
            ArtworkCacheLock.acquire()
            globals()["ArtworkCache"][0] += ContentSize
            globals()["ArtworkCache"][1][Payload] = (f"HTTP/1.1 200 OK\r\nServer: Emby-Next-Gen\r\nConnection: close\r\nContent-Length: {ContentSize}\r\nContent-Type: {ContentType}\r\n\r\n".encode(), BinaryData, ContentSize)
            ArtworkCacheLock.release()
            del BinaryData
            DelayedContentLock.acquire()
            globals()['DelayedContent'][QueryData['DelayedContentId']][0] = ArtworkCache[1][Payload][0] + ArtworkCache[1][Payload][1]
            DelayedContentLock.release()
            xbmc.log(f"Loaded Delayed Content for {Payload}", 1) # LOGDEBUG
        else:
            # We still have the lock from above, get the data before another thread possibly overwrites it in the cache for over 100MB
            # but make a copy so we don't hold the lock during the full send
            toSend = ArtworkCache[1][Payload][0] + ArtworkCache[1][Payload][1]
            ArtworkCacheLock.release()
            xbmc.log(f"EMBY.hooks.webservice: Load artwork data from cache: {Payload}", 1) # LOGDEBUG
            client.send(toSend)

        return

I think you did absolutely right. I'll double check and add i in the the code.

Thanks for the great support, I really appreciate support from programmers.

[seashell 35]

1 minute ago, quickmic said:

I think you did absolutely right. I'll double check and add i in the the code.

Thanks for the great support, I really appreciate support from programmers.

You're welcome.  Of course the locks need to be used in the other functions that use ArtworkCache and DelayedContent.

And if you really want to do it right if the threading library in kodi is new enough put timeout on the acquire calls, check that you actually got lock and fail appropriately if you did not.   You can also put the critical sections in try-except blocks to make sure the lock gets released so one thread having an error doesn't bring the whole thing down with the others waiting on a lock that will never be released.

[quickmic 1352]

2 minutes ago, seashell said:

You're welcome.  Of course the locks need to be used in the other functions that use ArtworkCache and DelayedContent.

And if you really want to do it right if the threading library in kodi is new enough put timeout on the acquire calls, check that you actually got lock and fail appropriately if you did not.   You can also put the critical sections in try-except blocks to make sure the lock gets released so one thread having an error doesn't bring the whole thing down with the others waiting on a lock that will never be released.

Will check, but I use the low level apis from "_thread" like in the "queue.py" file

 

[seashell 35]

2 minutes ago, quickmic said:

Will check, but I use the low level apis from "_thread" like in the "queue.py" file

 

For _thread it says 3.2 or greater for timeout.  And the syntax might be slightly different for lock acquire/release I didn't check. 

lock.acquire(blocking=True, timeout=-1)

Without any optional argument, this method acquires the lock unconditionally, if necessary waiting until it is released by another thread (only one thread at a time can acquire a lock — that’s their reason for existence).

If the blocking argument is present, the action depends on its value: if it is False, the lock is only acquired if it can be acquired immediately without waiting, while if it is True, the lock is acquired unconditionally as above.

If the floating-point timeout argument is present and positive, it specifies the maximum wait time in seconds before returning. A negative timeout argument specifies an unbounded wait. You cannot specify a timeout if blocking is False.

The return value is True if the lock is acquired successfully, False if not.

Changed in version 3.2: The timeout parameter is new.

Changed in version 3.2: Lock acquires can now be interrupted by signals on POSIX.

 

[seashell 35]

Here's a quick attempt at the matching send_delayed_content function for the counter system with locks.

 

def send_delayed_content(client, DelayedContentId):
    global DelayedContent, DelayedContentLock
    xbmc.log(f"EMBY.hooks.webservice: send_delay_content: {DelayedContentId}", 1) # DEBUGINFO
    DelayContentLock.acquire()
    if DelayedContentId in DelayedContent:
        DC = DelayedContent[DelayedContentId][0].copy() # Copy because it's a pointer whose content could be changed by another thread outside the lock
    else:
        DC = None
    DelayContentLock.release()
    if DC:
        xbmc.log(f"EMBY.hooks.webservice: Content available: {DelayedContentId}", 1) # DEBUGINFO

        if DC == "blank":
            send_BlankWAV(client, DelayedContentId)
        else:
            client.send(DC)

            DelayContentLock.acquire()
            # Things could have changed by other threads since the check at the top so check again
            if DelayedContentId in DelayedContent:
                DelayedContent[DelayedContentId][1] -= 1
                if DelayedContent[DelayedContentId][1] <= 0:
                    del DelayedContent[DelayedContentId]
            DelayContentLock.release()

        return True
    else:
        xbmc.log(f"EMBY.hooks.webservice: Delayed content not found {DelayedContentId}", 3) # LOGERROR
        client.send(sendNoContent)
        return True

    return False

Edited April 26 by seashell

[quickmic 1352]

7 minutes ago, seashell said:

Here's a quick attempt at the matching send_delayed_content function for the counter system with locks.

 

def send_delayed_content(client, DelayedContentId):
    global DelayedContent, DelayedContentLock
    xbmc.log(f"EMBY.hooks.webservice: send_delay_content: {DelayedContentId}", 1) # DEBUGINFO
    DelayContentLock.acquire()
    if DelayedContentId in DelayedContent:
        DC = DelayedContent[DelayedContentId][0].copy() # Copy because it's a pointer whose content could be changed by another thread outside the lock
    else:
        DC = None
    DelayContentLock.release()
    if DC:
        xbmc.log(f"EMBY.hooks.webservice: Content available: {DelayedContentId}", 1) # DEBUGINFO

        if DC == "blank":
            send_BlankWAV(client, DelayedContentId)
        else:
            client.send(DC)

            DelayContentLock.acquire()
            # Things could have changed by other threads since the check at the top so check again
            if DelayedContentId in DelayedContent:
                DelayedContent[DelayedContentId][1] -= 1
                if DelayedContent[DelayedContentId][1] <= 0:
                    del DelayedContent[DelayedContentId]
            DelayContentLock.release()

        return True
    else:
        xbmc.log(f"EMBY.hooks.webservice: Delayed content not found {DelayedContentId}", 3) # LOGERROR
        client.send(sendNoContent)
        return True

    return False

I'm thinking if the Locks must be also assigned to a dict with key DelayedContentId. A single lock object might break http-multi-queries...

Edited April 26 by quickmic

[seashell 35]

16 minutes ago, quickmic said:

I'm thinking if the Locks must be also assigned to a dict with key DelayedContentId. A single lock object might break http-multi-queries...

Why?  DelayedContentId is a local variable everywhere I've seen it.  You only need to lock the global variable.  If you're worried about pointer effects either hold the lock a bit longer or make a copy like I did with the string.

You can point me to the code in question and I can give you a better opinion, but I think what I've done will work fine in the two functions I modified however they are called.

I'm probably done for tonight though in terms of code review or writing more.

[quickmic 1352]

20 minutes ago, seashell said:

Why?  DelayedContentId is a local variable everywhere I've seen it.  You only need to lock the global variable.  If you're worried about pointer effects either hold the lock a bit longer or make a copy like I did with the string.

You can point me to the code in question and I can give you a better opinion, but I think what I've done will work fine in the two functions I modified however they are called.

I'm probably done for tonight though in terms of code review or writing more.

DelayedContentId is the key for the global var "DelayedContent".

It's more or less the http playload -> ""DelayedContentId": Payload.replace("/", "")"

The lock should only intercept parallel queries for the same payload.

e.g.

2 images are queried parallel but complete different ones, the lock should not block those, otherwise it would result in a stack.

[seashell 35]

4 minutes ago, quickmic said:

DelayedContentId is the key for the global var "DelayedContent".

It's more or less the http playload -> ""DelayedContentId": Payload.replace("/", "")"

The lock should only intercept parallel queries for the same payload.

e.g.

2 images are queried parallel but complete different ones, the lock should not block those, otherwise it would result in a stack.

No the point of the locks is to protect any modification or reaction to global variables.  They are fine to use on non-problematic content because they are brief and have minimal impact on the speed the code runs at.  The only lock I worry about about is the scan 100MB of data while under lock.  No need to special case things if their normal impact is small which i believe it will be.

Edited April 26 by seashell