fix for reported problem on some SONY TV's while content browsing / searching

[ik666]

This might solve the issue reported in: #2376. I hope the TV doesn't make consecutive search requests.

https://www.universalmediaserver.com/forum/viewtopic.php?f=9&t=14807&p=44349#p44349

[SubJunk]

Thanks!

[valib]

@ik666 can you check my changes if it is working for you?

[valib]

This is not implemented and there is a question if we need it. Folders are not stored in the database.

[ik666]

Yes you're right. In the beginning, I was searching only for "FILES" type. But then the more concrete types were necessary and "FILES" became obsolete. Thanks for figuring out.

[SubJunk]

It's a good point but it presents a problem with the search approach, since it is valid for renderers to search for folders, isn't it?

[ik666]

It is valid to search for "object.container.storageFolder", but the current implementation ignores this upnp:class yet.

Since the SearchRequestHandler will not create an URL-Link of type TYPE_FOLDER, we should not actually have an issue here in this concrete java class (in discovering the DLNAResource which is not implemented).

[valib]

@ik666 @SubJunk we should aware users that all this implementation is working correctly only if the database is enabled and the UMS will fully scan shared folders. Otherwise the result will be not relevant.

[SubJunk]

@valib yes. At this point I think the database and startup scan are essential for UMS to run properly in many ways. That's why those settings are hidden by default. People should not disable them unless there is a bug to avoid, and I am always working hard to fix bugs

[ik666]

@valib I had a quick check. Your changes are all right. I had no issues.

[SubJunk]

The user replied that it didn't work, should I make a new build with these latest changes @valib ?

[ik666]

@SubJunk I'm not sure if the fix didn't work, or if there are "still errors". To prevent spamming, @valib changed the loglevel to TRACE, so that this error shouldn't pop up in the "general" log any more. Maybe a rebuild is needed for changes to take effect.

Since the code calls the pre 10.6 logic in case of any error (RequestV2: around 1112) ,

		try {
			return searchRequestHandler.createSearchResponse(requestMessage, mediaRenderer);
		} catch (Exception e) {
			LOGGER.trace("error transforming searchCriteria to SQL. Fallback to content browsing ...", e);
			return browseHandler();
		}

I'm wondering if the origin of this issue lies within the UPnP search code or not ?

Besides that, the users error message shows a search for object.item.imageItem. Since UMS doesn't store any images, delivering an empty container should actually be fine, too (current implementation on master branch) ?!

[ik666]

@SubJunk @valib
ok, I think I see the problem. The TV makes a search for top 14 available video items (log file around line 4085). This leads actually not to an exception, therefore not entering the pre 10.6 logic. The result is 0, which might be an error. I could add SQL queries in TRACE log to dig deeper into this.

However, even if the search delivers top 14 videos in SearchResponse I personally think, this is not the intended user experience, since the user might rather like to browse the media library (beginning with the root element) instead of looking at unsorted top 14 video files ...

One solution could be disabling UPnP search support by a configuration entry for specific TV types by delegating the call directly to browseHandler(). I haven't done any TV specific configuration stuff yet ... can one of you give me a hint, or just implement the configuration entry if you'd like to move in this direction?

[Nadahar]

Besides that, the users error message shows a search for object.item.imageItem. Since UMS doesn't store any images, delivering an empty container should actually be fine, too (current implementation on master branch) ?!

I'm not following the logic of the search implementation here, but I don't understand your claim that "UMS doesn't store any images". Images should be "stored" in the database like other media files, but their metadata is stored in a different way. For audio and video, a few selected metadata is stored in the database tables. For images, all metadata is stored in ImageInfo which is then serialized and stored in the database. These metadata thus isn't available to make queries in the database, but the image media files should still "be there".

The reason for storing the metadata this way is that there are so many different metadata available for different formats that making a column for all of them would mean that most would be null/blank for most images. If some of these properties are needed for SQL queries, columns could be created for those specific metadata.

[SubJunk]

@ik666 it might be some kind of "smart" filtering system for the TV. For example on PS3 it has different entry points for media servers depending on what you want - video, audio or image. In the case of PS3 it just uses browse requests and then ignores the results it doesn't want, but maybe there is a similar user experience on the TV. The fact it requests 14 videos may be because of pagination or some quick-preview feature or something. Maybe when it's working we can ask the user to provide photos of the interface.

Anyway it's up to the TV to do what they want with our API, and it's up to us to fulfil the requests that we report to support. I think it's kinda exciting when renderers do unconventional things with DLNA/UPnP :)

Maybe we can just try to fulfil the request in a basic way and see where that leads? Maybe the renderer will send a browse request too after we fulfil the search requests. What do you think?

[SubJunk]

These are possibly related as well. Not fully confirmed but the timing lines up and I don't think we have done other big changes lately:

https://www.universalmediaserver.com/forum/viewtopic.php?f=9&t=14247&p=44366#p44366
https://www.universalmediaserver.com/forum/viewtopic.php?p=44364#p44364

Maybe the best thing for now would be to disable the search capability and work on it some more, what do you think @ik666 ? That way I can do a release very soon with that disabled and then we can continue to work on it for a future release

[SubJunk]

Unfortunately I need to go away from this computer for a few days or more for a COVID-19 lockdown so I think I will do a release of master while I have time, I was hoping to wait until we had a fix for this but I'll get the release ready now

[valib]

@SubJunk has anybody test my last commit?

[SubJunk]

@valib no, I wasn't sure if it was ready for testing yet. Do you have the ability to do Windows builds? I won't be able to for at least 4 days unless I figure out building from CI which I have been meaning to do...

[ik666]

@ik666 it might be some kind of "smart" filtering system for the TV. For example on PS3 it has different entry points for media servers depending on what you want - video, audio or image. In the case of PS3 it just uses browse requests and then ignores the results it doesn't want, but maybe there is a similar user experience on the TV. The fact it requests 14 videos may be because of pagination or some quick-preview feature or something. Maybe when it's working we can ask the user to provide photos of the interface.

Anyway it's up to the TV to do what they want with our API, and it's up to us to fulfil the requests that we report to support. I think it's kinda exciting when renderers do unconventional things with DLNA/UPnP :)

Maybe we can just try to fulfil the request in a basic way and see where that leads? Maybe the renderer will send a browse request too after we fulfil the search requests. What do you think?

I don't mind to figure out why the resultset has no entries. I'll add some SQL trace code. Nevertheless, I somehow doubt that the TV will make BrowseRequests after the SearchRequest if the resultset is >0 ... we'll see.

[ik666]

Besides that, the users error message shows a search for object.item.imageItem. Since UMS doesn't store any images, delivering an empty container should actually be fine, too (current implementation on master branch) ?!

I'm not following the logic of the search implementation here, but I don't understand your claim that "UMS doesn't store any images". Images should be "stored" in the database like other media files, but their metadata is stored in a different way. For audio and video, a few selected metadata is stored in the database tables. For images, all metadata is stored in ImageInfo which is then serialized and stored in the database. These metadata thus isn't available to make queries in the database, but the image media files should still "be there".

The reason for storing the metadata this way is that there are so many different metadata available for different formats that making a column for all of them would mean that most would be null/blank for most images. If some of these properties are needed for SQL queries, columns could be created for those specific metadata.

You're right. I made wrong assumptions. I'm uncertain how interesting an image search might be for the end user ...

[ik666]

These are possibly related as well. Not fully confirmed but the timing lines up and I don't think we have done other big changes lately:

https://www.universalmediaserver.com/forum/viewtopic.php?f=9&t=14247&p=44366#p44366
https://www.universalmediaserver.com/forum/viewtopic.php?p=44364#p44364

Maybe the best thing for now would be to disable the search capability and work on it some more, what do you think @ik666 ? That way I can do a release very soon with that disabled and then we can continue to work on it for a future release

I can disable the search, however I doubt it will have any effect on the "massive" or "very large" id issue, since the search implementation uses it's own resource loading (doesn't rely on the PMS object cache). Actually, I had the same experience while doing the search implementation. Even though same files were found, new ID's were generated for the same object, leading to misses in resolving thumbnails. That's the reason, why I implemented a "database-id based resource locator" circumveting this issue.
@Nadahar you might remember our discussion on this point.

[ik666]

Lastet commit disables UPnP search.

[ik666]

Unfortunately I need to go away from this computer for a few days or more for a COVID-19 lockdown so I think I will do a release of master while I have time, I was hoping to wait until we had a fix for this but I'll get the release ready now

That's bad :( I wish you the best ...

[SubJunk]

Thanks, it's ok it's just my government is very responsive - we had one case of infection in New Zealand and the country is in lockdown. It's very cool and keeps us safe.

Ok I can include that disabling in the next release (I am in the middle of releasing 10.10.0 now, which I built hours ago) and then hopefully we can get searching back into the program soon

[ik666]

@SubJunk Good to know ...

But I did a c&p error :(( . Please do also merge GIT 3615470 or this complete branch. to disable search support.

[SubJunk]

@valib do you think we should merge this?

[ik666]

Sorry, I was some days off. I think it looks nice now :)

[SubJunk]

Ok cool, I think it's good now but I just want to check with @valib that he is ok with it too

[valib]

@SubJunk I am still testing it because I have a problem with the test case. For me the test is not working in Eclipse but it pass when I use mvn site. Strange. Maybe the problem in Eclipse setting.

[SubJunk]

@valib I just tested it and it passed too. Do we need to document that for Eclipse users?

[Nadahar]

This is not my business, but I really think it sounds like a bad idea to "document" that it's broken under Eclipse and move on. I don't know exactly what doesn't work under Eclipse, but I find it very likely that this problem can manifest under other circumstances as well and isn't the "fault" of Eclipse.

It the problem is the tests, I'd bet it is a threading/concurrency issue. Eclipse runs the tests somewhat differently than Maven, and I've experienced tests that fail only under Eclipse and vice versa. If you tweak the Maven testrunner's thread handling, I'm pretty sure you'll get that to fail too.

A much better solution would be to fix the concurrency bug IMO, if this is the situation.

[valib]

Now it works in Eclipse. It takes some time but it works for me.

[SubJunk]

@Nadahar sure it's your business, you're a long-time code contributor and supporter of us on the forum, your thoughts are always very welcome here :)

@valib nice work fixing it, thanks!

[ik666]

I'm back from holidays and I'm glad to see you guys having everything under control :)