Bug report + patch fix for mishandling of transcoding and format specific resolutions.
Bug report + patch fix for mishandling of transcoding and format specific resolutions.
Currently, UMS fails to properly support format specific high resolution streaming. As an example, take the playstation config extract below which specifies a high resolution format specific format. With this config, if an mp4 h264 file is provided with a higher res than 1080p it will be transcoded by default even if it is fully compliant.
MaxVideoWidth = 1920
MaxVideoHeight = 1080
MediaInfo = true
Supported = f:mp4 v:h264 a:aac-lc|ac3|lpcm|mp4a w:3840 h:2160 m:video/mp4
Supported = f:mp4 v:h264 a:he-aac n:2 w:3840 h:2160 m:video/mp4
The first part of the patch (to DLNAResource.java) addresses the problem which is that the render default resolution MaxWidth,MaxHeigh (1920,1080) is applied to all valid streams even if they have already passed a higher resolution check. This check is what is causing valid high resolution streams to be flagged as invalid and hence to be transcoded.
With the first patch the resolution test for formats with default resolution has been removed and so the second patch adds the default width and height to all format specific parameters and then allows this to be overwritten by the user provided format specific values. Pretty nasty hack in this one ... but I didn't want to be invasive into FormatConfiguration as that seems like quite nice code compared to the rest.
I didn't submit it as a pull request as it kind of feels like there should be better ways to solve the problem and there are similar default based issues with other format specific parameters like bit rate plus the width / height functions which return invalid sizes are used elsewhere. That would likely involve some sort of re-write or code restructure involving the renderer type hierarchy and I'll totally understand if the code owner isn't in the mood for that - probably add as many bugs as it fixes.
Let me know if you'd prefer this as a pull request.
MaxVideoWidth = 1920
MaxVideoHeight = 1080
MediaInfo = true
Supported = f:mp4 v:h264 a:aac-lc|ac3|lpcm|mp4a w:3840 h:2160 m:video/mp4
Supported = f:mp4 v:h264 a:he-aac n:2 w:3840 h:2160 m:video/mp4
The first part of the patch (to DLNAResource.java) addresses the problem which is that the render default resolution MaxWidth,MaxHeigh (1920,1080) is applied to all valid streams even if they have already passed a higher resolution check. This check is what is causing valid high resolution streams to be flagged as invalid and hence to be transcoded.
With the first patch the resolution test for formats with default resolution has been removed and so the second patch adds the default width and height to all format specific parameters and then allows this to be overwritten by the user provided format specific values. Pretty nasty hack in this one ... but I didn't want to be invasive into FormatConfiguration as that seems like quite nice code compared to the rest.
I didn't submit it as a pull request as it kind of feels like there should be better ways to solve the problem and there are similar default based issues with other format specific parameters like bit rate plus the width / height functions which return invalid sizes are used elsewhere. That would likely involve some sort of re-write or code restructure involving the renderer type hierarchy and I'll totally understand if the code owner isn't in the mood for that - probably add as many bugs as it fixes.
Let me know if you'd prefer this as a pull request.
Re: Bug report + patch fix for mishandling of transcoding and format specific resolutions.
Thanks for your effort. If it is okay with you please make the PR.
Re: Bug report + patch fix for mishandling of transcoding and format specific resolutions.
Hi kinkyjohn,
I agree, please submit the pull request and then we can hopefully merge It would also be nicer for us to leave any comments we have on there
I agree, please submit the pull request and then we can hopefully merge It would also be nicer for us to leave any comments we have on there