After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 165193 - Patch for ov51x v4ljpegsrc
Patch for ov51x v4ljpegsrc
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins
git master
Other Linux
: Normal enhancement
: 0.8.8
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-01-25 15:37 UTC by Jan Schmidt
Modified: 2005-02-03 22:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for v4ljpegsrc (3.91 KB, patch)
2005-01-25 15:39 UTC, Jan Schmidt
none Details | Review
new gstv4ljpegsrc.[ch] files (3.98 KB, application/x-gzip)
2005-01-26 02:11 UTC, Jan Schmidt
  Details

Description Jan Schmidt 2005-01-25 15:37:52 UTC
Attaching a patch that adds a v4ljpegsrc derived from v4lsrc. I had to do this
as a separate element because V4L has no format for Jpeg output frames, so the
driver simply returns JPEG data the the framebuffer when you set up for RGB frames. 

Examples of ov51x camera are the Sony EyeToys.

Putting the patch here because I think it's a bit dirty - anyone else got a
better way to implement it? At the moment it derives from v4lsrc and replaces
the link and get functions on the pads behind the parent class' back.
Comment 1 Jan Schmidt 2005-01-25 15:39:15 UTC
Created attachment 36508 [details] [review]
Patch for v4ljpegsrc
Comment 2 Ronald Bultje 2005-01-25 18:07:17 UTC
I only see atches to existing files, can you attach the new files, too?
Comment 3 Jan Schmidt 2005-01-26 02:11:10 UTC
Created attachment 36535 [details]
new gstv4ljpegsrc.[ch] files

silly me, here's the new files to go along with the patch
Comment 4 Ronald Bultje 2005-01-26 08:46:22 UTC
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.
Comment 5 Ronald Bultje 2005-01-26 09:01:19 UTC
Additional note: do these cams have an extra flag set? It'd be nice to test this
in the _open function in v4l_calls.c, so that we don't open the device as a
wrong type. Also, this'd make device probing in Cupid continue to work.
Comment 6 Jan Schmidt 2005-01-26 10:32:49 UTC
Thanks for the review. Regarding gst_v4lsrc_try_capture in the link function,
that comes from v4lsrc so if there's a better way to do it, there's 2 spots to
improve.

Unfortunately, I can't see anything we can use to distinguish the cameras from
others, other than the string in the driver name, and even that doesn't help
because the same driver also does ov511/ov518 cameras that produce uncompressed
data.

I've written to the driver author to ask him, but I think basically it is just
required that the user knows their using one of these cameras and uses a
different element, otherwise they'll just get pretty flickering static for output.

Comment 7 Ronald Bultje 2005-02-03 22:10:06 UTC
Hm, crap. Ohwell. Anyway, you applied it, so let's close this one.