Add Dolby Vision data pass through #90
Conversation
|
Thank you. How do I test if it's working? Any public sample video and steps to verify in player like MPC? |
|
Google -> "dolby vision sample" ;) |
|
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? |
|
Are you kidding? O_o |
|
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. |
meaning it's some kind of fake DoVi w/o any meta-data... dunno
it's right in the first message: "Latest MPC Video Renderer supports Dolby Vision" ;) |
| if (const ATL::CComQIPtr<IMediaSideData> sideData(outSample); sideData != nullptr) { | ||
| processSourceFrameIters[0]->second.hdrSideData->WriteTo(sideData); | ||
| bool ok = false; | ||
| if (sourceFrameNb >= 0) { //must be always set with AVS 3.6+ |
There was a problem hiding this comment.
In which case would sourceFrameNb differ from processSourceFrameIters[0]->first? When we emplace the source frame in AddInputSample(), the key to the _sourceFrames entry is _nextSourceFrameNb, which is also what you put into the frame prop. So in theory they should always be the same. Which means, processSourceFrameIters[0]->second.hdrSideData should be the same as iter->second.hdrSideData.
|
Please read #87 carefully... |
|
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 |
|
Hi @chainikdn. Are you still working on this? |
|
Close for inactivity. |
|
If you don't want to merge this - I'm good with my own build. |
|
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. |
|
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.
No I'm not. |
#87