jellyfin / jellyfin-webos

WebOS Client for Jellyfin
https://jellyfin.org
Mozilla Public License 2.0
623 stars 60 forks source link

[Feature] Report no support for mkv, if DV is supported #159

Open Schaka opened 1 year ago

Schaka commented 1 year ago

There is currently a problem, where non of the LG TVs will display any DV video stream correctly (colors are wrong) when using mkv containers as opposed to mp4.

What I'm looking for is an option to report no mkv support altogether to the Jellyfin server if DV is supported. Ideally, this would be dependent on whether we're trying to play DV media in the first place. But as a second option, just having a setting that will incorrectly report no mkv support to the server to force transmuxing to mp4 would be a great option. I know I certainly would make use of this.

I wasn't able to find anywhere in the code where I could change this myself or I would've submitted a PR straight away.

dmitrylyzo commented 1 year ago

DOVI is added here with no condition.

Instead, something like the following can be added (after other "VideoRange" blocks).

if (option.supportsDolbyVision) {
    hevcVideoRangeTypes += '|DOVI';
}

But, looking at #109 (DOVI only in MP4), we need to add a separate CodecProfile for MP4 + HEVC. The question is, how to detect that the client only supports DOVI in certain containers?

Schaka commented 1 year ago

I would consider it a hack, but I don't see an issue with the WebOS reporting this by setting it so manually. As far as I understand, WebOS uses jellyfin-web to access the server but still receives supported formats from the client itself (in this case, the TV through WebOS DeviceProfiles(?)). It doesn't seem to be supported on ANY WebOS device. If no devices are supporting mkv+DV, simply disabling it for WebOS might be an option.

Considering there seem to be problems on almost all devices (Shield, AndroidTV, web, Roku, webOS) with playing DV streams in mkv containers, maybe it's worth it to just implement a new setting for the client like [beta] disable mkv support (this forces transmux to a supported container.

Bear with me, as I'm still trying to understand how the CodecProfiles work. But it seems to check whether mp4 is supported? In that case, checking for mp4 support and only then disabling mkv might be an option too.

dmitrylyzo commented 1 year ago

I would consider it a hack, but I don't see an issue with the WebOS reporting this by setting it so manually. As far as I understand, WebOS uses jellyfin-web to access the server but still receives supported formats from the client itself (in this case, the TV through WebOS DeviceProfiles(?)).

The device profile is constructed by profileBuilder in jellyfin-web. The app is passing some hints to it. For example, supportsDolbyVision.

It doesn't seem to be supported on ANY WebOS device. If no devices are supporting mkv+DV, simply disabling it for WebOS might be an option.

A quick search didn't give an answer. At least, I don't think much testing was done when the VideoRangeType condition was introduced. Tizen and webOS were added as an assumption.

Considering there seem to be problems on almost all devices (Shield, AndroidTV, web, Roku, webOS) with playing DV streams in mkv containers, maybe it's worth it to just implement a new setting for the client like [beta] disable mkv support (this forces transmux to a supported container.

I was thinking of a customizable device profile - enabling/disabling codecs/containers. This could help get around some problems, but it also makes them harder to troubleshoot. No one is currently working on it.

Bear with me, as I'm still trying to understand how the CodecProfiles work. But it seems to check whether mp4 is supported? In that case, checking for mp4 support and only then disabling mkv might be an option too.

Each CodecProfile has a list of containers and a list of codecs, and is applied only if it matches the container+codec combination of the current media.

It might look like this:

{
    "Type": "Video",
    "Codec": "hevc",
    "Container": "mp4",
    "Conditions": [
        {
            "Condition": "EqualsAny",
            "Property": "VideoRangeType",
            "Value": "SDR|HDR10|HLG|DOVI",
            "IsRequired": false
        }
    ]
},
{
    "Type": "Video",
    "Codec": "hevc",
    "Container": "-mp4",
    "Conditions": [
        {
            "Condition": "EqualsAny",
            "Property": "VideoRangeType",
            "Value": "SDR|HDR10|HLG",
            "IsRequired": false
        }
    ]
}
Schaka commented 1 year ago

Would this be overwriting the existing values under the given conditions? Pushing DOVI for tizen and webOs would be correct theoretically, if the given conditions in your example restrict the VideoRangeType further.

As I see it, your example profile could be pushed as additional conditions to hevcCodecProfileConditions around L#932 where some special handling is already being done.

Could you point me towards where these conditions are evaluated so I can get a better grasp at how they're being evaluated?

So far, this fix seems simpler than I had anticipated. Really appreciate you taking the time explaining it to me.

dmitrylyzo commented 1 year ago

As I see it, your example profile could be pushed as additional conditions to hevcCodecProfileConditions around L#932 where some special handling is already being done.

Not as an additional - it has to replace this because all the conditions will be checked and if any of them fail, the media will be treated as unsupported. That's why I made a mutually exclusive container filter in the example.

Could you point me towards where these conditions are evaluated so I can get a better grasp at how they're being evaluated?

Condition filter: https://github.com/jellyfin/jellyfin/blob/2acae258b808a5d404e5800932839c20fef419df/MediaBrowser.Model/Dlna/StreamBuilder.cs#L1146

Condition check: https://github.com/jellyfin/jellyfin/blob/2acae258b808a5d404e5800932839c20fef419df/MediaBrowser.Model/Dlna/StreamBuilder.cs#L1148 https://github.com/jellyfin/jellyfin/blob/2acae258b808a5d404e5800932839c20fef419df/MediaBrowser.Model/Dlna/StreamBuilder.cs#L1127

kvaster commented 11 months ago

I've tried to do such fix for myself as a hack and now I'm really confused.

First of all it seems that matroska format is always remuxed/transcoded currently, except it is really webm (no hevc support in this subset). It is not recognized as supported container for DirectPlay. On web you always receive following profile:

    DirectPlayProfile
    {
      Container: "mkv",
      AudioCodec: "aac,mp3,ac3,eac3,mp2,pcm_s16le,pcm_s24le,opus,flac,vorbis",
      VideoCodec: "h264,hevc,mpeg2video,vc1,vp9,av1",
      Type: Video
    },

But ffprobe returns matroska,webm as a container. So mkv profile can't be used. I think this needs a fix.

Going further with Dolby Vision - almost all videos are not recognized as DOVI VideoTypeRange. Let's take a look on MediaStream.GetVideoColorRange. This functions checks color_transfer and immidiatelly returns HDR10 as VideoTypeRange in case smpte2084.

I've checked my files and I found only few DOVI files of 5.1 profile and all other DOVI files where of 8.1 profile. This are all base layer + rpu dovi streams. For profile 5.1 ffmpeg does not return color_transfer and returns dvhe as a codeg_tag (also returns proper codec_tag for all audio streams), for profile 8.1 ffmpeg returns color_transfer=smpte2048 and no codec_tag at all for all streams including audio.

So, the question is when we need to say that file is VideoTypeRange.DOVI ? Profile 5.1 is really only DOVI and can't be properly played as HDR10, but profile 8.1 can be played as HDR10 and reported as HDR10 right now instead of DOVI. Also not always you can remux mkv to mp4 and play. i.e. you can't use TrueHD streams in mp4 or some of subtitles formats can't be converted to be used in mp4. And you should 'choose' between remuxing to mp4 without such unsupported extra data.

kvaster commented 11 months ago

Some additional notes: it seems that codec_tag is reported only for mp4 files. For mkv files codec_tag is always broken. Also after converting DOVI mkv to mp4 it shows codec_tag as hev1.

dmitrylyzo commented 11 months ago

But ffprobe returns matroska,webm as a container. So mkv profile can't be used. I think this needs a fix.

It worked before, iirc.

Are you using unstable server? The format should supposedly have been normalized here, but since it is comma separated, it is not. Related commit: https://github.com/jellyfin/jellyfin/commit/47aa07c3424ce0041e0a79eea1ab7f6621485b94 10.8.x looks fine: https://github.com/jellyfin/jellyfin/blob/4f6edd9c3c8f3c9ad2ce5c590d8e1fc185afa3ea/MediaBrowser.MediaEncoding/Probing/ProbeResultNormalizer.cs#L233-L248

A quick workaround for web is to add ,matroska here.

kvaster commented 11 months ago

I'm using latest git. It worked previously - probably was broken some time after 10.8.x. And I've already added workaround for web. But not I'm stucked with ffmpeg :) It seems that ffmpeg does not preserve dovi metadata when remuxing to hls stream. It works OK when remuxing the whole file from mkv to mp4, but it doesn't work for hls (either if it is created from mp4 or from mkv).

kvaster commented 11 months ago

I've created a ticket in ffmpeg: https://trac.ffmpeg.org/ticket/10490