Making streaming and transcoding logic better configurable #9

Merged
merged 12 commits into from Jan 20, 2012

3 participants

@Raptor399
PS3 Media Server member

The logic that decides whether to stream or transcode makes use of the method ps3compatible().
@StreamHD made a proposal and a patch to move away from that method in favor of a more general method isCompatible(). I took his patch and expanded upon it, resulting in this pull request.

Raptor399 added some commits Jan 14, 2012
@Raptor399 Raptor399 Moving away from Format.ps3compatible() (thanks, StreamHD!) a39a218
@Raptor399 Raptor399 Added comments, rewrote inline conditions. be31ecd
@Raptor399 Raptor399 Moved isCompatible() implementation to Format.
Removed the overrides of it in all formats. Also verified that the
PS3.conf correctly mimicks the behavior of ps3compatible(), meaning the
new isCompatible() should correctly mimicks the behavior of
ps3compatible() for the PS3 in particular.

Other renderers may notice different behavior now that the hardcoded
behavior of ps3compatible() is gone. That is a side effect of the
newly won possiblity to truly configure compatibility per renderer.
2194f3d
@Raptor399 Raptor399 No format should be able to override the isCompatible() behavior. 975a67c
@Raptor399 Raptor399 Removed outdated comments 86e2af0
@Raptor399 Raptor399 Reverting back to original method name. 72b4b39
@Raptor399 Raptor399 Added unit tests for Format and isCompatible() 62f04f7
@Raptor399 Raptor399 Removing skip() override introduced in 562fdb8.
The method interfered with the correctness of isCompatible() and its
backwards compatibility with ps3compatible().

On top of that the comment seemed to want to force transcoding, where
in reality returning true ended up forcing streaming. To remove this
paradox and to restore control to each individual renderer
configuration the method has been removed, bringing the code in line
with all other formats.
7675911
@Raptor399 Raptor399 Added tests for backwards compatiblity of isCompatible() dc3e72a
@Raptor399 Raptor399 Updated CHANGELOG b04509f
@Raptor399 Raptor399 Fixed isCompatible() for web images not appearing. ef3bfda
@taconaut
PS3 Media Server member

Lets see if I get this right: Up to now it was only compatible to stream (remux?) video streams if the renderer was a ps3 (according to some hard coded parameters). With this change it is now possible to configure natively supported streams for any renderer in the renderer configuration.
What is being checked? Container (mkv, avi...)? Video tracks (x264, MPEG-2...)? Audio tracks (mp3, AAC, DTS...)? Subtitle tracks (sub, idx...)?
If a compatible video and audio track is being found inside an usupported container, will they be remuxed inside a compatible one?
Additionally, does this allow to specify if a renderer supports multiple audio/subtitle track inputs which then can be selected directly on the renderer (like you would for a DVD)?

Many questions, but that's the (core) part of pms I've always been trusting in without really understanding how it decides such things :)

If the above assumptions are right, it sounds like a great change!

@Raptor399
PS3 Media Server member

You're right in that this pull request is touching the core of the logic that decides whether to stream or transcode.

Part of that logic currently is a method named "ps3compatible()". As its name suggests, it locks down part of the logic to one particular renderer: the PS3.

Locking abilities down to one renderer is not necessary at all; we have a good system in place for each renderer to define its own abilities. This pull request addresses that and unlocks it all - whilst staying backwards compatible with the PS3. Other renderers may notice a change in the choice PMS makes for transcoding formats; if they do, it can now be fixed in the renderer.conf.

Nothing is changed in the surrounding PMS code, so if you could specify stuff before, you still can. If PMS parsed information before, it still does. This only fixes the hardcoded PS3 booleans that were being used.

If you want to know more about the checks: take a look at FormatConfiguration.match(). It shows exactly how the "Supported" lines are matched to determine incompatibilities. This is based on the information that was parsed by DLNAMediaInfo. Unless exceptions (StreamExtensions, TranscodeExtensions, NoTranscode etc.) were defined in the configuration.

@taconaut
PS3 Media Server member

Thanks for the clarification, I'll have a look at the methods you've linked to.

@taconaut taconaut and 1 other commented on an outdated diff Jan 16, 2012
src/main/java/net/pms/dlna/DLNAResource.java
if (!PMS.getConfiguration().isMencoderDisableSubs()) {
hasSubsToTranscode = (PMS.getConfiguration().getUseSubtitles() && child.isSrtFile()) || hasEmbeddedSubs;
}
boolean isIncompatible = false;
-
// FIXME: Remove PS3 specific logic to support other renderers
@taconaut
PS3 Media Server member
taconaut added a line comment Jan 16, 2012

All these FIXME comments can be removed before accepting the pull request ;)

@Raptor399
PS3 Media Server member
Raptor399 added a line comment Jan 16, 2012

Hahaha! Indeed!
I'll scan the code for remaining unnecessary FIXME's and remove them in an additional commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@StreamHD

Raptor399, once again, thanks for the significant contributions!

taconaut, I will try fill up some gaps for you:

I first noticed problems with my Samsung in PMS 1.50.0. I wrote a "supported = mkv" line in my conf and couldn't get some videos to stream (only to remux/transcode). Now all options are available.

What is being checked? Container (mkv, avi...)? Video tracks (x264, MPEG-2...)? Audio tracks (mp3, AAC, DTS...)? Subtitle tracks (sub, idx...)?

For easier reading, look at the PS3.conf. Any media that can be defined with a "supported =" line can be checked for renderer compatibility.

It should be possible to add support to PMS for more containers, video/audio formats, etc. if mediainfo.dll supports them.

If a compatible video and audio track is being found inside an usupported container, will they be remuxed inside a compatible one?

Remuxing should work the same as before. All my renderers work as expected.

does this allow to specify if a renderer supports multiple audio/subtitle track inputs which then can be selected directly on the renderer (like you would for a DVD)?

Yes, but it does this indirectly. For now, if you want to use embedded aud/sub tracks then you need to stream the video. This patch ensures that the [No Transcode] folder appears correctly for supported video files.

I think there is potential to do a selective aud/sub remux (for bandwidth savings and ofcourse subs) but that's a different project ;)
With native support for external subs, you can have sync problems if you remux the video.

@taconaut
PS3 Media Server member

Thanks for explaining, StreamHD!
So, this change allows to stream supported content as defined in the renderer configuration file whereas previously it was only possible to do it for the ps3 (according to hardcoded parameters)? It will only change the behaviour of content streamed as is without any processing, right?
As I currently only use the ps3 as a renderer where no subtitles are supported and plan to change the TV for one natively supporting mkvs through DLNA I'm trying to figure out what it will be able to do.
On the fly muxing of external subtitles would be a nice addition if it is technically possible.

@StreamHD

I think it is important to note that prior to PMS v1.21.1, there was no concept of renderer.conf (not even ps3.conf) or MediaInfo. PMS then relied on ps3compatible() for assessing format compatibility. The changes introduced in v1.21.1 gave PMS a renderer-independant architecture, which has proven successful to now. This patch makes use of this architecture to remove some of the remnants of the old architecture.

this change allows to stream supported content as defined in the renderer configuration file

correct

previously it was only possible to do it for the ps3 (according to hardcoded parameters)

Partially correct. It was possible if you defined skip transcode or stream extensions options. If you use Media Info then only movies without subtitles will stream. Movies with subtitles are transcoded. This is the normal behaviour of PMS and is unaffected by this patch.

It will only change the behaviour of content streamed as is without any processing, right?

Behaviour shouldn't change much actually:
For renderers using MediaInfo, you can expect to see the [NO TRANSCODE] option appear for your video if the renderer supports it. Previously, it would appear if the PS3 supported it (which is not useful if the renderer has different native support to a PS3).

Everything else should behave the same as usual.

As I currently only use the ps3 as a renderer where no subtitles are supported and plan to change the TV for one natively supporting mkvs through DLNA I'm trying to figure out what it will be able to do

External subs are simple so little can go wrong. PMS sends a response pointing to the subtitle and the renderer will use it. The problem is if you don't have playback info (like during transcoding) the subtitles can go out of sync.

It has embedded audio support, and FF/FR/time skip which only works when streaming.

Not good bandwidth on the 100Mbps port though and the UI is weak. The built-in upscaling engine and format support are great which makes up for those flaws.

@Raptor399 Raptor399 merged commit c761b08 into ps3mediaserver:master Jan 20, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment