[gstreamer-bugs] [Bug 165193] Patch for ov51x v4ljpegsrc

[bugzilla-daemon at bugzilla.gnome.org]

Please DO NOT reply to this by email. All additional comments should be made in
the comments box of this bug report.

 http://bugzilla.gnome.org/show_bug.cgi?id=165193
 GStreamer | gst-plugins | Ver: HEAD CVS





------- Additional Comments From Ronald Bultje  2005-01-26 03:46 -------
static gfloat gst_v4lsrc_get_fps (GstV4lSrc * v4lsrc);

appears a copy from the v4lsrc version, just expose it, don't copy.

_link: don't throw a GST_ELEMENT_ERROR() on caps negotiation failure, because it
is not fatal. It is fatal when no caps was negotiated after all caps nego is
over, which can be multiple runs. This may me a bug in v4lsrc as well, so fix it
there too. ;). I'm not very sure whether using _try_capture in _link is a good
idea, because it takes rather long, but ohwell... We'll think of optimization later.

fourcc = GST_MAKE_FOURCC ('R', 'G', 'B', ' ');

??? That's five-year old API. :). It seems unused, too, so remove it.

_get: using 2 bytes of shared hardware memory for the size of the data? Whoever
thought of this should be burned alive, POSIX has explicit return values for
that. Not your fault, but you get the point. ;). For timestamp copying, use
gst_buffer_stamp(). Also:

    unsigned short *read_size = ((unsigned short *)(GST_BUFFER_DATA (buf)));
    jpeg_size = (int)(read_size[0]) * 8;

I don't know what you're trying to do here, but you should probably use
GST_READ_UINT16_LE(). This is non-portable. Check that the given size is inside
the buffer, else error out, just for safety (people could try this on their TV
card, which would give interesting segfaults).

General: I think it's OK. It may not be the nicest API I've seen around, but the
code in the element is nice 'n simple.

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