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

MJPEG support for released package #43

Open
7675t opened this issue Sep 20, 2017 · 6 comments
Open

MJPEG support for released package #43

7675t opened this issue Sep 20, 2017 · 6 comments

Comments

@7675t
Copy link
Contributor

7675t commented Sep 20, 2017

I believe Motion JPEG(mjpeg) cameras are not supported in current released ROS packages. Is it correct?

I think the remedy is to release libuvc newly (v.0.0.6). It will resolve all problem related to mjpeg.
I'm not sure the backward compatibility of libuvc to v.0.0.5. Are there any difficulties to release libuvc v0.0.6 ? @ktossell

Thank you.

@k-okada
Copy link
Contributor

k-okada commented Sep 20, 2017

I think releasing newer version of libuvc and libuvc_ros into new distro, like M-Turtle is ok. I (or other ROS Orphaned Package Maintainer) can do that. But if you want to use this package with libuvc v0.0.6 in current distribution, such as Kinetic, I'm afraid that I can not say anything about an regression due to the update, because I have not looking into the source tree. So I'm afraid that I can take the responsibility for that change and I need to some other maintainer to do that.

Assuming if you want use this in Kinetic, I think re-release libuvc_camera with enabling libjpeg dependency is very tricky but sounds ok to me. It just add new function into binary and there might be little change for regression. The code path used by current user will not change, so there might be no harm.

BTW, why libuvc_camera requires libuvc > 0.0.5 ? I could not find any difference between v0.0.5 and latest for mjpeg features.
https://github.com/ktossell/libuvc/commits/v0.0.5/src/frame-mjpeg.c
https://github.com/ktossell/libuvc/commits/master/src/frame-mjpeg.c
And I did not remember until I pushed Blame button, but it is me who added this change #41. May be this is my fault. uvc_mjpeg2rgb is already available on v0.0.5 if libuvc is compiled with libjpeg and even if you released v0.0.6 without libjpeg, it goes error, so we should change the code like.

+#if libuvc_VERSION     >= 00005 /* version >= 0.0.5 and have jpeg */ (How can we ensure that we have libjpeg on libuvc, running nm libuvc.so | grep mjpeg on cmake???)

Or assuming v0.0.5 have libjpeg support.

So, what I can do is.

  1. add libuvc/libuvc@351203e into v0.0.5 of libuvc at release repository
  2. fix package.xml at https://github.com/ros-drivers-gbp/libuvc-release/blob/ad4a3653018eaddbea2201d57313b62239e3157e/jade/package.xml#L13
  3. re-release libuvc as v0.0.5-1
  4. fix 'libuvc_camera requires libuvc > v0.0.5'
  5. release new version of libuvc_camera

But I do not have mjpeg device and did not sure if this change really enabling mjpeg support, so @7675t can you test this on your device?

  1. compile libuvc v.0.0.5 with jpeg support (need to apply libuvc/libuvc@351203e)
  2. re-compile libuvc_camera with ' libuvc_VERSION >= 00005'
  3. check if this works for MJPEG camera and also normal (YUYV???) camera.

@7675t
Copy link
Contributor Author

7675t commented Sep 21, 2017

I understand we should not do major changes for already released packages. So true, but I feel pessimistic while I tried.

  1. compile libuvc v.0.0.5 with jpeg support (need to apply libuvc/libuvc@351203e)

No problem to build. Say this is libuvc(v0.0.5-1).

  1. re-compile libuvc_camera with ' libuvc_VERSION >= 00005'

I got errors like:

/home/tajima/Codes/theta_ws/src/libuvc_ros/libuvc_camera/src/camera_driver.cpp:338:65: error: ‘uvc_find_devices’ was not declared in this scope
     new_config.serial.empty() ? NULL : new_config.serial.c_str());

It is OK. libuvc_camera contains three 'libuvc_VERSION > 00005' lines. The error can be avoided by changing the line only on MJPEG.

However,

  1. check if this works for MJPEG camera and also normal (YUYV???) camera.

My camera is RICOH Theta S in MJPEG mode. I got error with libuvc(v0.0.5-1) like:

[ INFO] [1505950839.718105168]: Opening camera with vendor=0x5ca, product=0x2711, serial="", index=0
uvc_start_streaming: Not supported (-12)
[ INFO] [1505950839.719763562]: camera calibration URL: file:///tmp/cam.yaml
[ INFO] [1505950839.719813906]: Unable to open camera calibration file [/tmp/cam.yaml]
[ WARN] [1505950839.719833144]: Camera calibration file /tmp/cam.yaml not found.
[camera/mycam-2] process has died [pid 13994, exit code 255, cmd /home/tajima/Codes/theta_ws/devel/lib/libuvc_camera/camera_node __name:=mycam __log:=/home/tajima/.ros/log/1b9cab3a-9e5d-11e7-bbc7-0028f8c8dc14/camera-mycam-2.log].
log file: /home/tajima/.ros/log/1b9cab3a-9e5d-11e7-bbc7-0028f8c8dc14/camera-mycam-2*.log

With libuvc(master), it works fine.

And my laptop webcam works fine with both version (v0.0.5-1 and master) in uncompressed video mode(yuyv).

Unfortunately, concerning MJPEG support of libuvc, we need other changes we don't know from v0.0.5.
As @k-okada said, ROS package maintainer cannot fix it with confident.

I feel I should give up to include it in Kinetic. Maybe Lunar or Mutant...
But if we do so, we should fix the description in ROSWiki it doesn't support mjpeg.
And we need to wait for libuvc(v0.0.6) anyway.

@k-okada
Copy link
Contributor

k-okada commented Sep 21, 2017 via email

@7675t
Copy link
Contributor Author

7675t commented Sep 21, 2017

He added new tag very quickly, thanks @ktossell !
Now how can we do? I can check libuvc(v0.0.6) + libuvc_camera with my hardwares, but the variation is limited.

And it may happens after ROSCon.

In my opinion, ROS libuvc_camera users have used it without mpeg support, so mpeg support cannot be regressed. The other part (YUYV, RGB, etc) is basic part of libuvc, so the possibility of problem should be low if we check it with several hardwares.

@k-okada
Copy link
Contributor

k-okada commented Sep 21, 2017

I asked @ktossell to give @7675t write permission of release repository. Once you get that, let's release from L-Turtle. It only has 3% usages, so I think it is ok -> http://download.ros.org/downloads/metrics/metrics-report-2017-07.pdf

@7675t
Copy link
Contributor Author

7675t commented Sep 21, 2017

Sounds nice. Thank you!

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

No branches or pull requests

2 participants