[Bug 763026] dc1394: port to 1.X

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763026

--- Comment #5 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
> All usages seem correct to me then.
> Should GST_ELEMENT_ERROR be used in the set_property method (or some callee)
> if a property can not be set to the given value (e.g. set_prop_iso_speed)?

Not really. There isn't really a good way to communicate a set_property()
failure to the caller other than to just not change the property (they can
check with get_property() if it worked if they want to). This would be a
programmer error however, and it would be unusual to post an error message for
that.


> Ok, I think that the device provider/monitor can be added
> after merging this in, in a different bug/request.

Yes :)


> What type of property do you think is more suitable: string or 64 bit
> integral?

I would go for a GUID string. Presumably this is also something one might find
somewhere in some system tool that shows connected devices, no?



> > The change_state func should really go away.
> > Use GstBaseSrc::start/stop() (or open/close) instead.
> 
> Ok, does that mean that we agree on my point about the camera operation
> and the states transitions? It seems odd to me to start the transmission
> in the set_caps method, but I can not figure out any other way that would
> work.

I think so. I don't see what else you can do really, and it's a live source. It
sounds fine. We can still improve it later if we can think of a better way.



> > Maybe the iso-speed property should be an enum property instead?
> > 
> Would something like the following be ok?

Looks good to me, yes. I trust this means you agree it makes sense..


> Could you also confirm the doubt about the IYU2 format in my first point:
> Version 0.X maps DC1394_COLOR_CODING_YUV444 to IYU2 fourcc format,
> which is no longer defined in 1.X. It is Y444 format the right equivalent?

As far as I can tell IYU2 is a packed 4:4:4 format, but with component ordering
like U Y V U Y V whereas Y444 is for planar 4:4:4, so the closest would be the
v308 format. I think we just have to add a new video format for IYU2. Shouldn't
be too difficult, there's IYU1 already :)


> And a final question about how should I proceed to make the merging easier.
> Should I amend the last commit to produce a patch with the new changes,
> or create new commits and provide consecutive patches?

Please amend your original patch. Then once it's merged we can do follow-up
patches separately. Thanks!

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.