Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some compiler warnings #734

Merged
merged 3 commits into from
Jun 4, 2022
Merged

Conversation

atsampson
Copy link
Collaborator

@atsampson atsampson commented May 29, 2022

Some of QScopedPointer's API is deprecated in favour of std::unique_ptr in Qt 6.1; use std::unique_ptr instead.

Avoid a signed/unsigned comparison in ld-ldf-reader. Edit: with CI, this revealed a fault in the seeking logic - fixed.

Remove some dead code in ld-chroma-encoder. (I had to do some testing to convince myself it really wasn't needed; separate PR coming to add the test.)

@atsampson atsampson added enhancement ld-decode-tools An issue only affecting the ld-decode-tools labels May 29, 2022
@atsampson atsampson requested a review from simoninns May 29, 2022 00:59
@simoninns
Copy link
Collaborator

Failing CI @atsampson - please verify

Qt 6.1 deprecates some of QScopedPtr's API, and recommends using
unique_ptr instead.

(Note that since this code is built as C++11 with Qt 5, we can't use
C++14's std::make_unique yet, but that would be worthwhile in the
future.)
GCC warned about an signed/unsigned comparison here. After testing, this
revealed that the logic to work out how much of a frame to skip when
seeking wasn't correct, meaning it always output the whole frame
(although fortunately this doesn't make much difference for
ld-decode/ld-cut's purposes!).

Rewrite the code to avoid the signed/unsigned confusion and skip the
right amount of data.
lineLen wasn't being used, and isn't needed (checked by comparison with
BBC subcarrier-locked samples).
@atsampson
Copy link
Collaborator Author

This turned out to be another bug that fixing the comparison revealed - so I've included a fix for that as well...

@simoninns simoninns merged commit 185fe8e into happycube:master Jun 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ld-decode-tools An issue only affecting the ld-decode-tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants