Use QVector<quint16> rather than QByteArray for TBC fields and RGB output #418
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This avoids needing to reinterpret_cast to quint16 in all the places that work with field data; the only unsafe casts are now when the data is being read from or written to a file. It also avoids a few unnecessary copies of the data (e.g. several places where a non-const pointer was taken). It also fixes a few minor problems: - A couple of functions explicitly treated the TBC data as little-endian; it's now native-endian throughout. - In ld-dropout-correct, statistics could be reported for the wrong frame, because they were read from the new OutputFrame rather than the one being written out. - In ld-process-vbi, WhiteFlag was stepping by bytes rather than words through the samples (but since the samples should be either black or white, this shouldn't really make much difference).
This avoids several reinterpret_casts in the decoders, and simplifies the display code in ld-analyse.
Since SourceVideo is now non-copyable, the file and cache don't need to be allocated with new.
atsampson
added
enhancement
ld-decode-tools
simoninns
approved these changes
Jan 21, 2020
There was a problem hiding this comment.
Looks good to me; I pulled the changes and gave it some quick tests. ld-diffdod and ld-discmap are the two I'm actively working on, so I will commit your changes and then update my own branch to test. Don't think they will get any end-user use in the meantime.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a respin of a patch I started last year...
The general idea is to use (named typedefs of) QVector rather than QByteArray for 16-bit TBC and RGB samples. This substantially reduces the number of places where the tools need to use reinterpret_cast, and generally simplifies the code working with fields and decoded frames - the only remaining unsafe casts in the video tools are when doing file IO. ld-chroma-decoder gets the most benefit from this, but most of the tools are affected to some degree.
I'm reasonably confident in the chroma-decoder/dropout-correct/process-vbi changes, but my testsuite doesn't do anything much with diffdod/discmap yet - please check those changes look OK!
See the commit messages for a couple of other minor improvements rolled into this.
One more thing I spotted but haven't changed: several functions in ld-process-vbi compute the zero crossing point as
videoParameters.white16bIre - videoParameters.black16bIre- should that actually be(videoParameters.white16bIre + videoParameters.black16bIre) / 2(the midpoint)?