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

add UYVY support #357

Merged
merged 4 commits into from Dec 3, 2017

Conversation

Projects
None yet
4 participants
@adicarlo
Copy link

adicarlo commented Sep 21, 2017

Consider a camera like this:

Driver Info (not using libv4l2):
	Driver name   : stk1160
	Card type     : stk1160
	Bus info      : usb-0000:00:14.0-10.7
	Driver version: 4.12.6
	Capabilities  : 0x85200001
		Video Capture
		Read/Write
		Streaming
		Extended Pix Format
		Device Capabilities
	Device Caps   : 0x05200001
		Video Capture
		Read/Write
		Streaming
		Extended Pix Format
Priority: 2
Video input : 0 (Composite0: ok)
Video Standard = 0x00001000
	NTSC-M
Format Video Capture:
	Width/Height      : 640/480
	Pixel Format      : 'UYVY'
	Field             : Interlaced
	Bytes per Line    : 1280
	Size Image        : 614400
	Colorspace        : SMPTE 170M
	Transfer Function : Default
	YCbCr/HSV Encoding: Default
	Quantization      : Default
	Flags             : 
Streaming Parameters Video Capture:
	Frames per second: 29.970 (30000/1001)
	Read buffers     : 2

User Controls

                     brightness (int)    : min=0 max=255 step=1 default=128 value=128 flags=slider
                       contrast (int)    : min=0 max=127 step=1 default=64 value=64 flags=slider
                     saturation (int)    : min=0 max=127 step=1 default=64 value=64 flags=slider
                            hue (int)    : min=-128 max=127 step=1 default=0 value=0 flags=slider
                     chroma_agc (bool)   : default=1 value=1 flags=update
                    chroma_gain (int)    : min=0 max=127 step=1 default=40 value=47 flags=inactive, slider, volatile

Note the pixel format here is UYVY. Attempting to start capture with pygame.camera will give you an error like this:

    cam.start()
SystemError: ioctl(VIDIOC_S_FMT) failure: no supported formats

The attached patch allows pygame to make use of this sort of camera.

I have tested this and I get good output in RGB, and, I think, expected output in YUV, its hard to say, since it doens't look normal like RGB, but its consistent with what I get with other camers in YUV mode.

@adicarlo

This comment has been minimized.

Copy link
Author

adicarlo commented Sep 21, 2017

The test failures seem to be spurious.

@aszlig

This comment has been minimized.

Copy link

aszlig commented Oct 17, 2017

Damn, just found this PR after creating my own patch.

@adicarlo: It might make sense for your implementation to make the pixel format conversion functions a bit more DRY instead of just copying the yuyv_to_* functions (see my patch above, although even in my patch it would probably make sense to rename the yuyv_to_* functions to something more general).

@adicarlo

This comment has been minimized.

Copy link
Author

adicarlo commented Oct 18, 2017

@aszlig You raise a good point about DRY, but speaking of DRY, why is it that pygame needs to include these fundamental pixel format conversions at all? Surely there is an efficient C library version, perhaps in a library we're already using, with an implementation we can use. That would mean less code, easier maintenance, and we'd also pick up a lot of additional unimplemented pixel conversions.

This more minimal approach would benefit pygame more, I think, but its beyond what I wanted to embark upon.

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Nov 29, 2017

Closing/reopening to re-run the tests with fixes in master.

@takluyver takluyver closed this Nov 29, 2017

@takluyver takluyver reopened this Nov 29, 2017

@adicarlo

This comment has been minimized.

Copy link
Author

adicarlo commented Dec 2, 2017

@takluyver any issues or way I can help ? Or was this already merged? Github seems to say "no".

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Dec 2, 2017

The tests are passing now. :-) I'd like someone else to have a quick review of it, because I'm not very familiar with C code.

@aszlig

This comment has been minimized.

Copy link

aszlig commented Dec 3, 2017

@takluyver: Apart from the comment mentioned above, LGTM.

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Dec 3, 2017

Thanks @aszlig !

In terms of clarity, it seems better to me to have separate functions than to have a yuyv_to_rgb function handling both yuyv and uyvy. Though maybe the solution is to give it a clearer name. @adicarlo , what do you think?

@illume

This comment has been minimized.

Copy link
Member

illume commented Dec 3, 2017

Either are good if it makes it work on more cameras :)

👍 from me.

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Dec 3, 2017

OK, let's press the button - the code arrangement can always be improved in a separate PR.

@takluyver takluyver merged commit d8cef0d into pygame:master Dec 3, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.