Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend media receiver registrar compatibility #1919

Conversation

sc3141
Copy link
Contributor

@sc3141 sc3141 commented Feb 20, 2020

Certain media renderers require greater adherence to the MS-DRMND protocol than is provided by UMS' current simulation of X_AV_MS_MediaReceiverRegistrar service. I discovered this when attempting to connect a Denon AVR-4311CI to my UMS server version 9.2.

I investigated the problem, determined the cause, and implemented a solution. My investigation is documented in this UMS forum topic.

To fix the connection problem by MSDRM-shackled renderers, this PR does the following:

  • Extend the simulation of MS-DRM service, X_AV_MS_MediaReceiverRegistear (MRR), to event subscription.
  • Change the parent upnp event and control paths to the MRR service such that they match the other two services. (/upnp/event, /upnp/control).
  • Add provision of a service description file on the same path as other service description file
  • Change default renderer behavior to include the MRR service in the UMS descriptor (i.e. description/fetch). A renderer configuration option (next item) is provided in order to allow this behavior to be disabled.
  • Add a new renderer configuration option, simulateMRR, which enables/disables exposure of MRR to a renderer (default: true)
  • Retain renderer predicate function, isXbox360(), due to specialized service which extends beyond the scope of MRR

This PR does not add a renderer configuration file for the Denon AVR-4311CI. I assume that incorporation of a new renderer is preferred to be done as a separate issue.

…tear (MRR), to event subscription.

- Motivation: some receivers (e.g. Denon AVR-4311CI) require authorization via an MRR server in order to allow access to the content of a DLNA media server.
- Change the parent upnp event and control paths to the MRR service such that they match the other two services. (/upnp/event, /upnp/control).
- Add provision of a service description file on the same path as other service description file
- Change default renderer behavior to advertise and provide the MRR service 'simulation' to renderers
- Add a new renderer configuration option, simulateMRR, which enables/disables exposure of MRR to a renderer (default: true)
  Retain renderer predicate function, isXbox360(), due to specialized service which extends beyond the scope of MRR

* src/main/external-resources/renderers/DefaultRenderer.conf
  document new option, SimulateMRR

* src/main/java/net/pms/configuration/RendererConfiguration.java
  add predicate function for new option, isMRRSimulated()
    default value of true, preserves legacy behavior w/ regards to Xbox 360 and
    opens the possibility that connection to MRR-needy renderers might work w/o
    user needing to be aware of the new option.

* src/main/resources/UPnP_AV_X_MS_MediaReceiverRegistrar_1.0.xml
  add service descriptor file for MRR

* src/main/java/net/pms/network/Request.java
* src/main/java/net/pms/network/RequestV2.java
  change MRR service paths to descriptor file and endpoints for upnp control and eventing
  adhere to MS-DRMND protocol w/ respect to response to subscription requests
…d MRR compatibility, such that the phrase 'Windows Media Connect' was being appended to the server name.

This was caused by the new default behavior which included simulation of MRR service to renderers. This in turn activated the legacy behavior for Xbox 360 for which the UMS server name was modified.

This commit reverts to the -legacy- behavior such that the server name is only decorated for the Xbox 360.

Note that special service to the Xbox 360 is activated by the presence of the 'xbox 360' in the renderer name. Any renderer which includes this phrase in its name via the config option, 'RendererName', will obtain the same 'special' service.
valib
valib previously approved these changes Feb 20, 2020
…ys 'Universal Media Server', because the previous commit related to media recevier registrar never incorporated the configuration file setting for non-Xbox360 renderers.

* src/main/java/net/pms/network/Request.java
* src/main/java/net/pms/network/RequestV2.java
  refactor chunk of code involved in the synthesis of the UMS friendly name.
@SubJunk
Copy link
Member

SubJunk commented Feb 21, 2020

@sc3141 thanks for this contribution, are you still working on it or do you think it is ready to be merged?

@sc3141
Copy link
Contributor Author

sc3141 commented Feb 21, 2020

Sorry, @valib, I found/fixed a bug which I introduced which regarded the setting of the friendlyName. Interesting phrasing by github, 'dismissed' ... all I did was push a commit.

@sc3141
Copy link
Contributor Author

sc3141 commented Feb 21, 2020

@SubJunk, yes I think it's ready. FriendlyName now is working as intended, which is that it only gets decorated with 'Windows Media Connect' for the Xbox 360 i.e. the (legacy behavior).

If you would like any follow-on work regarding the interaction of friendlyName with MRR authorization, just let me know.

I'm very impressed with UMS. I hope this PR helps a new group of users to enjoy it as much as I have.

@SubJunk
Copy link
Member

SubJunk commented Feb 23, 2020

@sc3141 it would be great to have more contributions from you, thanks in advance. Master is failing because of a third party so I'll fix that now and merge this

@SubJunk SubJunk merged commit 5558948 into UniversalMediaServer:master Feb 23, 2020
@SubJunk
Copy link
Member

SubJunk commented Feb 23, 2020

@sc3141 this is merged now and will be in the release I'm preparing today. Thanks!
It would be cool to see automated testing added for this to prevent future regressions. I wonder if this test file could be extended to perform that kind of check https://github.com/UniversalMediaServer/UniversalMediaServer/blob/master/src/test/java/net/pms/configuration/RendererConfigurationTest.java#L65

@sc3141
Copy link
Contributor Author

sc3141 commented Feb 25, 2020

@sc3141 it would be great to have more contributions from you, thanks in advance. Master is failing because of a third party so I'll fix that now and merge this

Thank you for the kind words, @SubJunk. I'm sure I'll be wanting to selfishly add another personally desired feature ;-). Seriously, I'm looking forward to my next contribution. First, I think I need to spend some more time test driving the server on my network.

@sc3141 this is merged now and will be in the release I'm preparing today. Thanks!
It would be cool to see automated testing added for this to prevent future regressions. I wonder if this test file could be extended to perform that kind of check https://github.com/UniversalMediaServer/UniversalMediaServer/blob/master/src/test/java/net/pms/configuration/RendererConfigurationTest.java#L65

I will take a look at it. I understand the importance of unit tests on a project such as this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants