quietvoid / dovi_tool

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

inject-rpu doesn't replace existing RPU in video file but inserts a new one (discarded by the player) #108

Closed zfeher closed 2 years ago

zfeher commented 2 years ago

First of all thank you for this handy tool.

I found a bug (feature?) when I tried to fix a cropped encoded video with set active area offset, and it didn't work (black bars stayed). I extracted the RPU from the demuxed encoded video (with --crop option but also tried editing the RPU) and injected that back to the encoded video. This had no effect, no matter how I changed the active area offsets.

I got curious and did some testing on the tool. The first problem I've found that it cannot extract the rpu from the injected video. This lead to repeating the injection and see what happens. The file size always got bigger i.e. the RPU was injected multiple times somehow (the root problem). And the first original one is the one that the TV player read, and discarded the rest.

This bug can lead to new features like remove-rpu, replace-rpu etc which can be handy when we want to fix/edit a video with existing RPU. Or inject-rpu could do the replacement (silently, with a warn, etc) if existing RPU is found.

quietvoid commented 2 years ago

The demux command can be used to remove RPUs.

I am aware that existing RPUs break injection, and so far it has been intended. Users should be aware of what is present in the files they use.

quietvoid commented 2 years ago

Implemented a warning when injecting and the input video contains an RPU in the first 100MB read. And when injecting, existing RPUs will not be written and thus wil be replaced by those injected.

zfeher commented 2 years ago

Thank you for suggesting the demux command, it solved the problem I have.

Thanks for the inject-rpu fix as well, it will be better user experience I think. Users can do wild/accidental things sometimes if allowed by the program.

Another idea came to my mind which could be nice to have feature one day:

quietvoid commented 2 years ago

On the fly cropping (even editing if possible, maybe not all kind) without demuxing a video with existing RPU. Might improve performance.

This is already supported with convert. Just use the --crop flag. I don't understand how it would have anything to do with performance.

Implementing "on the fly" edits is not something I'm interested in working on.

zfeher commented 2 years ago

Ok, understood. Thanks for pointing out the solution again.

By performance I meant the extra demuxing: writing to storage takes time and space. But thankfully convert, and then crop already works without this overhead.