GNOME Bugzilla – Bug 694155
Rygel does not support create-child-container
Last modified: 2013-04-02 10:07:48 UTC
This can be seen from the missing DLNACaps "create-child-container" and the fact that calling CreateObject with a container simply does not work. Here's an example of the error I see in the Rygel logs: (rygel:20652): Rygel-WARNING **: Failed to create item under 'DLNA.ORG_AnyContainer': No items in DIDL-Lite from client: '<DIDL-Lite xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:upnp="urn:schemas-upnp-org:metadata-1-0/upnp/" xmlns:dlna="urn:schemas-dlna-org:metadata-1-0/" xmlns="urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/"><container id="" parentID="DLNA.ORG_AnyContainer" restricted="0" dlna:dlnaManaged="00000001"><dc:title>Mark</dc:title><upnp:class>object.container.album</upnp:class></container></DIDL-Lite>' Note that Rygel advertises the "content-synchronization" capability and hence is obliged to support "create-child-container" as well.
I've just realised that there is already a related bug here: 684196: MediaExport plugin does not support CreateObject for child containers. However, as far as I can tell this bug is not just a MediaExport problem. Changes are also needed in Rygel core, e.g., the MediaServer3.xml file needs to be updated and unless I'm mistaken, changes will also be needed to the code in librygel-server to implement this. If I'm mistaken we can just close this as a duplicate.
No, that's correct. Things need to be changed server-side and implementation/plugin side
Created attachment 236780 [details] [review] server: Add create-child-container caps
Created attachment 236784 [details] [review] server: Add create-child-container caps
and some WIP to actually create containers: http://git.gnome.org/browse/rygel/tree/?h=wip%2Fcreate-container
*** Bug 684196 has been marked as a duplicate of this bug. ***
Created attachment 237507 [details] [review] server,media-export: Allow container creation
Created attachment 237512 [details] [review] server,media-export: Allow container creation
Review of attachment 237512 [details] [review]: ::: src/librygel-server/rygel-item-removal-queue.vala @@ +41,3 @@ } + public void queue (MediaObject item, Cancellable? cancellable) { Looks like this is an object removal queue instead of item removal queue. Maybe rename the class and parameter names accordingly? ::: src/librygel-server/rygel-item-creator.vala @@ +27,3 @@ /** + * Dummy implementation of Rygel.MediaContainer to pass on to + * Rygel.WritableContianer for cration. s/cration/creation/ @@ +207,3 @@ + // FIXME: This will take the last object in the DIDL-Lite, maybe we + // should limit it to one somehow. + this.didl_parser.object_available.connect ((didl_item) => { s/didl_item/didl_object @@ +423,3 @@ + this.didl_object.upnp_class); + + var didl_item = this.didl_object as DIDLLiteItem; I would move this line into the "if" below. Instead of checking if "didl_item != NULL" I would use "this.didl_object is DIDLLiteItem", because didl_item seems to be not used anywhere otherwise. @@ +458,3 @@ } + var item = this.object as MediaItem; I would move this line into the "if" below. @@ +633,2 @@ * + * When creating an object in the back-end via WritableContainer.add_item Mention WritableContainer.add_container? @@ +649,3 @@ try { + object = (yield container.find_object (this.object.id, + this.cancellable)) Remove the "as MediaItem" cast here. That will return NULL always if you create a container. ::: src/plugins/media-export/rygel-media-export-writable-db-container.vala @@ +82,3 @@ + default: + throw new ContentDirectoryError.BAD_METADATA + ("upnp:class not supported"); Maybe mention what upnp:class container has?
Created attachment 237525 [details] [review] (In reply to comment #9) > Review of attachment 237512 [details] [review]: > > ::: src/librygel-server/rygel-item-removal-queue.vala > @@ +41,3 @@ > } > > + public void queue (MediaObject item, Cancellable? cancellable) { > > Looks like this is an object removal queue instead of item removal queue. Maybe > rename the class and parameter names accordingly? Yes, forgot to rename that. Fixed. > > ::: src/librygel-server/rygel-item-creator.vala > @@ +27,3 @@ > /** > + * Dummy implementation of Rygel.MediaContainer to pass on to > + * Rygel.WritableContianer for cration. > > s/cration/creation/ Fixed. > > @@ +207,3 @@ > + // FIXME: This will take the last object in the DIDL-Lite, maybe we > + // should limit it to one somehow. > + this.didl_parser.object_available.connect ((didl_item) => { > > s/didl_item/didl_object Fixed. > > @@ +423,3 @@ > + this.didl_object.upnp_class); > + > + var didl_item = this.didl_object as DIDLLiteItem; > > I would move this line into the "if" below. Instead of checking if "didl_item > != NULL" I would use "this.didl_object is DIDLLiteItem", because didl_item > seems to be not used anywhere otherwise. Fixed. > > @@ +458,3 @@ > } > > + var item = this.object as MediaItem; > > I would move this line into the "if" below. The item is used later on. That's a bit dirty, I admit. Suggestions welcome. > > @@ +633,2 @@ > * > + * When creating an object in the back-end via WritableContainer.add_item > > Mention WritableContainer.add_container? > > @@ +649,3 @@ > try { > + object = (yield container.find_object (this.object.id, > + this.cancellable)) > > Remove the "as MediaItem" cast here. That will return NULL always if you create > a container. Fixed. > > ::: src/plugins/media-export/rygel-media-export-writable-db-container.vala > @@ +82,3 @@ > + default: > + throw new ContentDirectoryError.BAD_METADATA > + ("upnp:class not supported"); > > Maybe mention what upnp:class container has? Hm, how does that even compile? Fixed.
Review of attachment 237525 [details] [review]: (In reply to comment #10) > > I would move this line into the "if" below. > > The item is used later on. That's a bit dirty, I admit. Suggestions welcome. First five comments are suggestions. ::: src/librygel-server/rygel-item-creator.vala @@ +426,1 @@ if (resources != null && resources.length () > 0) { All code in this "if" is executed only when this.didl_object is DIDLLiteItem and this.object is MediaItem. Maybe you could put that "if" and next one inside "if (this.object is MediaItem && this.didl_object is DIDLLiteItem)" and remove then redundant checks. @@ +457,3 @@ + var item = this.object as MediaItem; + if (this.object is MediaItem) { That's the one you could put inside the "if" I mentioned earlier. @@ +471,3 @@ + this.object.uris.add (uri); + if (this.object is MediaItem) { + item.place_holder = true; If you move the ifs then you could s/item/(this.object as MediaItem). @@ +475,2 @@ } else { + var file = File.new_for_uri (item.uris[0]); No need for "item" here. "this.object" suits here as well. @@ +476,3 @@ + var file = File.new_for_uri (item.uris[0]); + if (this.object is MediaItem) { + item.place_holder = !file.is_native (); If you move the ifs then you could s/item/(this.object as MediaItem). @@ +644,1 @@ debug ("Waiting for new item to appear under container '%s'..", s/item/object @@ +653,1 @@ as MediaItem; That MediaItem cast is still there. I guess it is wrong. @@ +654,3 @@ } catch (Error error) { warning ("Error from container '%s' on trying to find newly " + "added child item '%s' in it", s/item/object
Created attachment 237542 [details] [review] wip: item refactoring
Created attachment 237543 [details] [review] wip: misc fixes
Attached some more patches on top of the old one. First one tries to address the item stuff, the other one are misc fixes I noticed while doing the first one.
Review of attachment 237542 [details] [review]: Ok.
Review of attachment 237543 [details] [review]: Ok.
Attachment 236784 [details] pushed as e39db18 - server: Add create-child-container caps
This still isn't working for me. I have two problems: 1. If I retrieve the properties of a folder e.g, Pictures in MediaExport, I see that object.container.storageFolder is listed in the create classes, but that the CreateChildContainer bit is not set in the DLNA Managed flag. 2. When I actually try to create a container, the request fails, with the error 'DLNA.ORG_AnyContainer': Flags that must not be set were found in 'dlnaManaged' I'm using a dlna:dlnaManaged value of "00000001", indicating that I would like to upload content to my newly created folder. This is a valid operation I think and so should be allowed by Rygel.
Created attachment 238287 [details] [review] server: Fix heck for invalid OCM flags This check has to be applied to items only, not all objects.
Created attachment 238288 [details] [review] server: Fix check for invalid OCM flags This check has to be applied to items only, not all objects.
Created attachment 238304 [details] [review] server: Set CreateContainer OCM flag This is not ideal and wrong in case of the Tracker plug-in, but the best we can do in freeze currently.
Attachment 238288 [details] pushed as d1a04eb - server: Fix check for invalid OCM flags Attachment 238304 [details] pushed as 00d9aec - server: Set CreateContainer OCM flag