Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#4442 closed defect (fixed)

upnpcdsvideo doesn't handle directories correctly

Reported by: David Asher <david.asher@…> Owned by: dblain
Priority: minor Milestone: 0.21
Component: upnp Version: unknown
Severity: medium Keywords:
Cc: Ticket locked: no

Description

There are at least 3 problems with the current svn head's (15373) handling of directories in upnpcdsvideo.cpp:

  1. When ProcessAll? is called (for Videos/0), it gets ALL videos instead of the videos whose parentid == STARTING_VIDEO_OBJECTID. Consequently the container returned for VideoRoot? reports too many children. I haven't provided a patch for this as it looks quite intrusive to fix, possibly requiring some refactoring around GetDistinctCount? or the handling of the root nodes.
  1. In UPnpCDSVideo::AddItem?, the child count is never filled in when a container is created. The attached patch fixes this.
  1. It looks like the support for multiple levels of directories is broken. A path like <root>/Sub1/Sub2/Vid1 doesn't produce the expected results. The IDs get kinda out of whack and extra levels of hierarchy keep getting created. I haven't fully worked this one out yet. (so no patch)

Attachments (1)

upnpvideofolder.patch (512 bytes) - added by David Asher <david.asher@…> 12 years ago.
patch to set child count when creating containers

Download all attachments as: .zip

Change History (11)

Changed 12 years ago by David Asher <david.asher@…>

Attachment: upnpvideofolder.patch added

patch to set child count when creating containers

comment:1 Changed 12 years ago by greg

Owner: changed from dblain to greg
Status: newassigned

comment:2 in reply to:  description ; Changed 12 years ago by greg

  1. When ProcessAll? is called (for Videos/0), it gets ALL videos instead of the videos whose parentid == STARTING_VIDEO_OBJECTID. Consequently the container

This shouldn't be too much to fix. It's just a modification to the GetCount?() routine. The code doesn't actually return ALL Videos, it just returns the count of all videos and the info for the children of the requested parentid.

So for example a call to Videos/0 in my setup returns

<NumberReturned?>12</NumberReturned?> <TotalMatches?>2795</TotalMatches?>

  1. In UPnpCDSVideo::AddItem?, the child count is never filled in when a container is created. The attached patch fixes this.

Thanks I'll look at that.

  1. It looks like the support for multiple levels of directories is broken. A path like <root>/Sub1/Sub2/Vid1 doesn't produce the expected results. The IDs get kinda

What exactly are you seeing ? I'm using it at least 5 levels deep (just tested again) with everything returning what I expect as far as I can tell. Are you seeing bad ID's or the wrong videos returned?

comment:3 in reply to:  2 Changed 12 years ago by greg

What exactly are you seeing ? I'm using it at least 5 levels deep (just tested again) with everything returning what I expect as far as I can tell. Are you seeing bad ID's or the wrong videos returned?

Actually Just to follow up my own reply, I do see some weirdness when a few levels deep. I'll check into it.

comment:4 Changed 12 years ago by David Asher <david.asher@…>

Wow. Thanks for the fast response.

As for item 1: The problem I had was GetDistinctCount? is called, but that is not a virtual function, so changing it will change it for all extensions. The SQL executed was:

select count(*) from upnpmedia;

what i think is desired is:

select count(parentid) from upnpmedia where parentid=1000000;

That CAN be done with GetCount?, but upnpcds.cpp is making the call in UPnpCDSExtension::ProcessAll?, which again is not a virtual function. So, I couldn't decide what was the best fix.

For item 3: I see the folder entry for Sub2, but when I enter it I get "no titles", and a new entry for Sub2 appears, which does have the titles in it. Very odd. If you need the XML result let me know and I'll try and get it for you. I got stuck when I realized I didn't really know what it SHOULD look like.

BTW, the client I'm using is a PS3 w/firmware v2.10, so ymmv.

comment:5 in reply to:  4 Changed 12 years ago by greg

Milestone: unknown0.21

As for item 1: The problem I had was GetDistinctCount? is called, but that is not a virtual function, so changing it will change it for all extensions. The SQL executed was:

I've got a fix for this, I just need to test it now.

For item 3: I see the folder entry for Sub2, but when I enter it I get "no titles", and a new entry for Sub2 appears, which does have the titles in it. Very odd. If you need the XML result let me know and I'll try and get it for you. I got stuck when I realized I didn't really know what it SHOULD look like.

There was a couple of problems which I've now commited fixes for which likely caused this so this should be fixed now.

comment:6 Changed 12 years ago by David Asher <david.asher@…>

Yep, I saw your updates. With the ObjectId? fix (item 3) and my count fix (item 2, which it appears you haven't commited yet) everything works for me.

I'm not sure why item 1 isn't causing problems. It definitely reports the wrong number of items for the VideoRoot? container. The PS3 seems to be ok with that, however.

comment:7 Changed 11 years ago by greg

Milestone: 0.210.22

comment:8 Changed 11 years ago by dblain

Milestone: 0.220.21
Owner: changed from greg to dblain

comment:9 Changed 11 years ago by dblain

Resolution: fixed
Status: assignedclosed

(In [16337]) * Removed client specific overrides from base classes.

comment:10 Changed 11 years ago by dblain

(In [16338]) * Removed client specific overrides from base classes.

Note: See TracTickets for help on using tickets.