quietvoid / dovi_tool

dovi_tool is a CLI tool combining multiple utilities for working with Dolby Vision.
MIT License
582 stars 57 forks source link

Correct Level 8 metadata length #105

Closed quietvoid closed 2 years ago

quietvoid commented 2 years ago

It seems level 8 metadata can have a variable length, thus it can contain more trim controls. Example:

  |  Extension block 8, Level 8, Length 13
     | target_display_index 1
     | trim_slope           2036
     | trim_offset          2060
     | trim_power           1987
     | trim_chroma_weight   2048
     | trim_saturation_gain 1985
     | ms_weight            2048
     | target_mid_contrast 2048
     | clip_trim 2011

Wouldn't be surprised if the 6-color trims for saturation/hue can also be in there.

saindriches commented 2 years ago

I have an imcomplete impl for variable blocks, and other things, it's in my fork now, could you take a look?

quietvoid commented 2 years ago

Looks like a lot of changes. I started fixing L8 but your approach is probably more complete.

I'll have a deeper look later.

saindriches commented 2 years ago

For now there are 3 variable blocks (L8 / L9 / L10), the L8 is the most complicated one, so I wrote some new functions about detecting length for these blocks, and keep the other code unchanged. Also the xml version is a number now. Parsing L11 and L254 is still incomplete. I'm not sure should I keep the L10 generation from json available with using the default_metadata_blocks or use new syntax for it.

quietvoid commented 2 years ago

Your required_bits logic looks pretty complicated. I'd rather store the block length and with that assume what fields were parsed and should be written.

It does increase the complexity for the end user and generate, but it already requires understanding the block structures.

saindriches commented 2 years ago

I see, that makes sense, so should I remove that parts and follow your style (to add a length field in struct, etc) and make a PR of these blocks, or let you do it?

quietvoid commented 2 years ago

I'm pulling your changes and adjusting right now, will keep you as co-author.

saindriches commented 2 years ago

Thanks. Since there is no ext block with id 0 (just so-called L0 metadata), I changed the reserved block id to 0 because I found that L255 does exist, maybe for DM debugging (fields are similar to L254), although I think it will not appear in official contents. Should we add it later?

quietvoid commented 2 years ago

Reserved blocks automatically fail the parsing anyways, so it doesn't really matter. The idea is to be able to support all possible blocks, and implement new ones when they're found.

Have you tried writing a level 255 block and get it recognized by the verifier? If so, then it could be useful to add support for.

saindriches commented 2 years ago

Yes, I have verified with it and make sure it matches.

manuelrn commented 2 years ago

It seems level 8 metadata can have a variable length, thus it can contain more trim controls. Example:

  |  Extension block 8, Level 8, Length 13
     | target_display_index 1
     | trim_slope           2036
     | trim_offset          2060
     | trim_power           1987
     | trim_chroma_weight   2048
     | trim_saturation_gain 1985
     | ms_weight            2048
     | target_mid_contrast 2048
     | clip_trim 2011

Wouldn't be surprised if the 6-color trims for saturation/hue can also be in there.

What does this mean exactly?

You mean level 8 metadata might contain more information than is currently being processed? In this case, then is it possible that some of the RPU metadata was lost when processed with current versions of dovi_tool?

quietvoid commented 2 years ago

In this case, then is it possible that some of the RPU metadata was lost when processed with current versions of dovi_tool?

No, otherwise the parsing would have errored.

manuelrn commented 2 years ago

In this case, then is it possible that some of the RPU metadata was lost when processed with current versions of dovi_tool?

No, otherwise the parsing would have errored.

Okay, so if dovi_tool has not shown errors it means that everything is fine, correct?

That is, if the situation that you mention occurs, dovi_tool would show an error instead of losing that part of the metadata, correct?

quietvoid commented 2 years ago

Yes that's correct. The reason I've noticed a problem is because someone got an error on a recent UHD disc and could provide the sample RPU.

For example in 1.4.3, when using mode=0 the extraction would fail with this:

Error: CM v4.0: Invalid metadata block. Block level 8 should have length 10

manuelrn commented 2 years ago

For example in 1.4.3, when using mode=0 the extraction would fail with this:

Error: CM v4.0: Invalid metadata block. Block level 8 should have length 10

I usually use mode 3 (to inject RPU WEB-DL in UHD), and I also use the editor to adjust the RPU (without specifying mode).

In these 2 cases would the result also be the same? (that error would be displayed and nothing from the metadata would ever be lost)

quietvoid commented 2 years ago

In these 2 cases would the result also be the same?

Yes.

manuelrn commented 2 years ago

In these 2 cases would the result also be the same?

Yes.

Ok, thanks for all your help as always!

ghost commented 2 years ago

AFAICT the 6-color trims go there too, but I don't know what format they're specified in. I'd assume each TID gets an array of 6 trims.

I've yet to see a V4 disc IRL though, and something seems bizarre about specifying a display with custom primaries as a target in the consumer versions since all consumer UHD devices that support DolbyVision also support one of the standard color spaces... unless one of those manufacturers of $50k multi-color laser projector with required $20k cooling systems has decided to start producing a DolbyVision add-on streaming box ($30k + $5k for required special toroidal power supply for the streaming box and paid Dolby some of the $95k in profit they're making on each sale to add it in for them so they can produce a super-impressive looking demo that matches something or another with the laser primaries (and which will never be included in any other video, from them or otherwise).

Then again, I'm not entirely sure how all of that data gets applied. I found at least one thing that said only two L2 targets are allowed / processed, and most commercial disks seem to have either BT.709/100 nit-only (and once again I lose the plot on why sRGB trims are included unless some severely terrible 4k screens somehow got DV certification) or that and either the 600 D65 or an unknown colorspace (not sure since TIDs aren't shown) 1000 nit target that's probably P3 because let's face it, nobody is going to include target trims for BT.2020... I found one website that mentioned something about TID 48 being P3 D65 and TID 49 being BT.2020, both @ 1000 nit though.