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

Disable 2020B unless Codec2 provides it. #257

Merged
merged 11 commits into from
Jul 14, 2022
Merged

Disable 2020B unless Codec2 provides it. #257

merged 11 commits into from
Jul 14, 2022

Conversation

tmiw
Copy link
Collaborator

@tmiw tmiw commented Jul 10, 2022

Due to build issues with v1.8.0 as reported in #255 and in-progress Codec2 API changes in master, 2020B must be disabled in the source code by default. This PR will allow 2020B to be enabled as an option if Codec2/LPCNet master are used for the build.

@tmiw tmiw requested a review from drowe67 July 10, 2022 08:45
build_linux.sh Outdated
CODEC2_BRANCH=master
LPCNET_BRANCH=master
CODEC2_BRANCH=v1.0.3
LPCNET_BRANCH=v0.2
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if we are checking out codec2 v1.0.3 ... how do we get 2020B?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user would need to edit this script to use master instead (or link against a Codec2 built manually from it).

@drowe67
Copy link
Owner

drowe67 commented Jul 10, 2022

@tmiw - the general approach looks fine 👍

@tmiw
Copy link
Collaborator Author

tmiw commented Jul 10, 2022

Hmm, slight problem. It appears lpcnet_get_hash() isn't in v0.2. That particular function was added in this commit. Should a v0.3 be released that's tagged at that commit? Is there another commit that should be used for v0.3 instead?

@drowe67
Copy link
Owner

drowe67 commented Jul 10, 2022

Hmm, that's curious - so we have been using that function for two years, and I guess several freedv-gui releases. How have we been building and packaging freedv-gui? Perhaps we have just been using LPCNet/master?

@tmiw
Copy link
Collaborator Author

tmiw commented Jul 10, 2022

Hmm, that's curious - so we have been using that function for two years, and I guess several freedv-gui releases. How have we been building and packaging freedv-gui? Perhaps we have just been using LPCNet/master?

Yeah, we've just been using master. I'd say to just continue using master except that there are likely API changes vs. 0.2, right?

@drowe67
Copy link
Owner

drowe67 commented Jul 10, 2022

Yes good point about the LPCNet API changing. That in turn implies the need for codec2/master, or, as you suggest, and earlier version of LPCNet, if we stick with codec2 1.0.3

Yikes, this is all getting messy 🤔

How about this:

  1. I'll look at codec2/master and see if I can make the minimum changes to make freedv_api.h compatible with existing applications - it was really just that one variable/structure.
  2. I wont worry about the 2020C work for now, will do that later.
  3. This implies we'll release 2020B into the wild now, which I'm not 100% happy with (as I'd like to compare it with 2020C first). Meh.
  4. We can then have a another try at a codec2 1.0.4 release, and do a 0.3 LPCNet release off LPCNet/master.

Does that plan work for you @tmiw ?

@tmiw
Copy link
Collaborator Author

tmiw commented Jul 11, 2022

Does that plan work for you @tmiw ?

That works. Once (4) is done, I'll update the build scripts again, confirm that all platforms build, then merge and release 1.8.1. 👍

@tmiw tmiw merged commit 2684f5b into master Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants