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 chroma rotation for phase comp decoder and make some json values that are integers actually output as int #787

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

oyvindln
Copy link
Contributor

I added a 10 degree offset to the chroma rotation in the phase compensating decoder as that seemed to fix phase offset, but after some digging it seems it was the samples that were off so it now correctly rotates 33 degrees which should give the correct result if the burst/color relationship in the sample is correct.

Also made some values that are supposed to be integers actually be output as integers to the json, round didn't change the datatype apparently so it was still outputting .0 int the json for those values.

Copy link
Owner

@happycube happycube left a comment

Choose a reason for hiding this comment

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

Is the -10 degree comment in comb.cpp still needed?

@oyvindln
Copy link
Contributor Author

oyvindln commented Aug 28, 2022

No can remove that, forgot that it was there.

One of the tests will fail afaik as the ld-chroma-encoder as the color bars on the sample the test generates don't line up correctly with the burst. (Visible on the vectorscope with the normal decoder if you modify the json to include the burst area)

@happycube
Copy link
Owner

@atsampson Does the encoder need correction here or is something wrong with this patch set?

from PIL import Image

img1 = Image.open('/home/cpage/ld-decode/git/testout/test.output-pc-ntsc2d-rgb.png')
img2 = Image.open('/home/cpage/ld-decode/i787//testout/test.output-pc-ntsc2d-rgb.png')

for i in [0, 110, 220, 330, 440, 550, 660, 700]:
print(i, img1.getpixel((i,200)), img2.getpixel((i,200)))

0 (189, 189, 189) (189, 189, 189)
110 (174, 174, 97) (167, 178, 95)
220 (82, 159, 45) (75, 160, 59)
330 (0, 173, 97) (0, 165, 135)
440 (62, 99, 62) (60, 99, 69)
550 (173, 0, 97) (176, 0, 59)
660 (82, 0, 46) (84, 0, 23)
700 (0, 0, 191) (16, 0, 195)

@oyvindln
Copy link
Contributor Author

The rotation part was fixed in @atsampson 's PR, but the json stuff still needs merging.

@happycube
Copy link
Owner

Can you update your side to include the rotation fix? Once that's done I'll merge it.

@oyvindln
Copy link
Contributor Author

oyvindln commented Sep 6, 2022

ok should be fixed now

@happycube happycube merged commit ae44a0c into happycube:master Sep 6, 2022
@oyvindln oyvindln deleted the upstream_stuff branch September 12, 2022 12:28
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