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

SCC: Handle non-matched SCC codes in SccWord.get_channel #427

Merged
merged 2 commits into from
May 14, 2024

Conversation

atsampson
Copy link
Contributor

@atsampson atsampson commented May 13, 2024

If is_code returns True but _find_code doesn't match the code, get_channel would crash rather than returning None.

(This was another case of a somewhat-mangled CC stream from a noisy LaserDisc via ld-decode; the invalid words were 1f38, 1d00, 1432, 1000.)

Closes #428

If is_code returns True but _find_code doesn't match the code,
get_channel would crash rather than returning None.
@palemieux
Copy link
Contributor

Feels like is_code() should return False if self.code is None.

@atsampson
Copy link
Contributor Author

That was my first thought, but it's used in _find_code which is called from the constructor. Maybe _find_code should just try and match regardless?

@palemieux
Copy link
Contributor

I see now, and agree that your patch fixes the immediate issue.

I see a broader issue with the semantics of is_code() and get_code() but probably should be fixed as a separate issue.

Can you add a test case that covers the issue that triggered the PR, and then we can merge.

Thanks!

@palemieux palemieux self-requested a review May 13, 2024 16:28
Check that it produces the correct result for some known codes, and that
none of the methods crash for all possible codes.
@palemieux palemieux merged commit c0f6929 into sandflow:master May 14, 2024
1 check passed
@atsampson
Copy link
Contributor Author

I've added a unit test for SccWord.get_channel, and another that tests that all the methods return something for all possible codes.

@atsampson atsampson deleted the channel-code branch May 14, 2024 15:08
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.

SCC: is_code() can return true even if the word is not a code
2 participants