CrendKing / avisynth_filter

DirectShow filters that put AviSynth and VapourSynth into video playing
MIT License
107 stars 8 forks source link

Add Dolby Vision data pass through #90

Closed chainikdn closed 6 months ago

chainikdn commented 8 months ago

https://github.com/CrendKing/avisynth_filter/issues/87

CrendKing commented 8 months ago

Thank you. How do I test if it's working? Any public sample video and steps to verify in player like MPC?

chainikdn commented 8 months ago

Google -> "dolby vision sample" ;)

CrendKing commented 8 months ago

As I understand, DV is an alternative implementation of HDR? And you are trying to pass the DV metadata to the renderer, right?

So what's not working? I downloaded some samples, I can't tell any difference between those DV videos and normal videos. Do I need to manually turn on HDR in Windows? Do I need specific video renderer (MPCVR? madVR?) Can you show me some screenshots before and after this fix?

chainikdn commented 8 months ago

Are you kidding? O_o https://4kmedia.org/lg-earth-dolby-vision-uhd-4k-demo/ Play it via MPC VR and via EVR. See the difference.

CrendKing commented 8 months ago

I do see the differences in color and contrast now with the video you provided. But I still can't tell any differences in some other DV videos such as https://4kmedia.org/lg-dolby-vision-uhd-4k-demo/. Not sure if that video being HDR is the key to trigger. Also it seems only MPC VR and mpv support DV out of box. madVR test build 181 doesn't (at least out of box).

All of these details are not straight obvious for someone uninitiated on the topic. That's why I asked you for setup instructions at beginning. Could have saved us both much time.

chainikdn commented 8 months ago

But I still can't tell any differences in some other DV videos

meaning it's some kind of fake DoVi w/o any meta-data... dunno

Also it seems only MPC VR and mpv support DV out of box

it's right in the first message: "Latest MPC Video Renderer supports Dolby Vision" ;)

chainikdn commented 8 months ago

Please read https://github.com/CrendKing/avisynth_filter/issues/87 carefully... In some cases I need to set different source frame for the interpolated frame. I.e. not the "left" one, but "right".

CrendKing commented 8 months ago

Gotcha. It's nice now both AVSF and VPSF pass the side data to output frame in consistent manner.

The only nitpick I have is that I personally don't like to use the output parameter in AVSF's PrepareOutputSample(). I would like to move the whole side data section in WorkerProc() to PrepareOutputSample(), just like what we do in VPSF. That way there is no need to output anything other than the bool result. Would you mind making that change, or I make that (cosmetic) change after I merge the PR? I could post it in a PR so you can review, if you want. Let me know.

CrendKing commented 7 months ago

Hi @chainikdn. Are you still working on this?

CrendKing commented 6 months ago

Close for inactivity.

chainikdn commented 6 months ago

If you don't want to merge this - I'm good with my own build.

CrendKing commented 6 months ago

Huh? In October, I asked you to make a small change before I merge the PR, and you since disappeared. I tried to not invade your space by changing it myself, since you blamed me for "you'll rewrite everything from scratch anyway".

Let me ask again: please move the whole side data section in WorkerProc() to PrepareOutputSample(), just like what we do in VPSF. That way there is no need to output anything other than the bool result. If you have your reason to not do it, please say it so we can discuss.

chainikdn commented 6 months ago

You asked if you can rewrite everything from scratch and obviously you can cause that's what you will do anyway. No need asking me if you can do this or not. Yes, you can :D This is why I didn't wanted to make any PRs at all, just give you all the necessary info so you could do it on your own from the beginning.

Are you still working on this?

No I'm not.