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

JPEG thumbnails not working #59

Closed
cdillow opened this Issue Mar 3, 2014 · 7 comments

Comments

Projects
None yet
2 participants
@cdillow

cdillow commented Mar 3, 2014

My Hi nice module thanks for making it. Is there an issue with the picamera module creating thumbnails in jpeg exif data or am I doing something wrong?

camera.capture(directory + imagename, format = 'jpeg', thumbnail = (320,240,80))

@waveform80

This comment has been minimized.

Owner

waveform80 commented Mar 4, 2014

I can confirm the issue but I'm baffled as to what's causing it at the moment. Raspistill is producing thumbnails correctly whilst picamera isn't and yet as far as I can see both are making the same set of underlying MMAL calls (in other words there may be something I'm missing here!).

I'll dig into this further at the weekend, but it's definitely something that needs fixing for the next version and adding to the test suite (although it'll probably add a dependency on exiftool to do so - PIL's exif support isn't great for this sort of thing).

@cdillow

This comment has been minimized.

cdillow commented Mar 4, 2014

OK Thanks for checking it out. let me know if I can help test. I need the thumbnail in order to switch from Raspistill to picamera.

@waveform80

This comment has been minimized.

Owner

waveform80 commented Mar 5, 2014

Eurgh - figured it out, but the fix is going to be horrid. For the sake of documenting the issue:

This is probably a "leaky abstraction" problem. The camera's ports have an "encoding" (nothing to do with the encoder we attach to them, but rather the raw format they produce data in). By default, picamera sets all ports, except the preview port, to YUV 4:2:0 encoding as this works for producing raw and cooked stills and video output. However, raspistill sets the still port encoding to "opaque". As far as I can tell this isn't a real encoding but rather a setting that effectively says to the downstream encoder, "instead of sending you the actual data, I'll send you a handle to the data".

Now, to the crux of the issue: when the still port is set to opaque encoding, all exif data is recorded correctly. When the still port is set to YUV 4:2:0 encoding, only explicitly set exif data (by default creation timestamp) is output - all other implicit exif data (focal length, exposure mode, et al) and thumbnail settings are ignored.

So why not set the still port to opaque? Because when we do, it breaks all raw captures (unsurprisingly; raw captures take output directly from the port without an encoder - if all they receive is a handle, not much is going to happen). Unfortunately, you can't change the encoding of a camera port without re-initializing the camera...

I'd like to fix this in such a way that one could still create an instance of PiCamera and then use it for jpeg output (with accurate exif data) or raw output without re-creating it. In other words, I'd like to fix this without breaking backwards compatibility with the published API, but to do so is going to require some nasty code under the covers that re-initializes the camera whenever we switch between cooked and raw image capture (on the still port; we never produce exif data on the video port). Given that we now support capture whilst recording, this is going to be a tad tricky!

@waveform80

This comment has been minimized.

Owner

waveform80 commented Mar 5, 2014

Ahhh ... and apparently the opaque encoding also breaks any usage of the resizer component. So - if a user calls capture() with use_video_port set to False (the default) and resize set to something other than None, do we raise an exception (because the resizer won't work with opaque encoding), or silently proceed with YUV 4:2:0 encoding but exclude most of the exif data?

I wonder if the splitter component works with the opaque encoding ... if it does we might at least be able to strap a splitter on the still port (like we already do on the video port) and then attach a raw or cooked encoder to differently configured output ports ... assuming the output ports of a splitter can differ in encoding to the input port?

@cdillow

This comment has been minimized.

cdillow commented Mar 5, 2014

Wow sounds tricky. Thanks for looking into this so fast. One of the things I like about PiCamera is the ease of resizing.

On Mar 4, 2014, at 7:10 PM, Dave Jones notifications@github.com wrote:

Ahhh ... and apparently the opaque encoding also breaks any usage of the resizer component. So - if a user calls capture() with use_video_port set to False (the default) and resize set to something other than None, do we raise an exception (because the resizer won't work with opaque encoding), or silently proceed with YUV 4:2:0 encoding but exclude most of the exif data?

I wonder if the splitter component works with the opaque encoding ... if it does we might at least be able to strap a splitter on the still port (like we already do on the video port) and then attach a raw or cooked encoder to differently configured output ports ... assuming the output ports of a splitter can differ in encoding to the input port?


Reply to this email directly or view it on GitHub.

@waveform80

This comment has been minimized.

Owner

waveform80 commented Mar 5, 2014

After a bit more testing it appears that a splitter's output ports can have a different encoding to the input port. Moreover, raw output from a splitter output port set to YUV 4:2:0 (while the corresponding splitter input is using opaque encoding) works nicely. Therefore, introducing a splitter for the still port, with one output set to opaque, and one set to YUV 4:2:0 is a potentially workable idea (avoids having to re-initialize the camera in order to switch port encoding).

That said, it appears there's no way the resizer will work with the opaque encoding. Throwing an exception when resize is used with use_video_port=False in capture() would potentially break existing code, so instead I'll just have to add a warning to the documentation that using resize with any of the capture methods will result in little/no exif data in the resulting output (and no thumbnails).

The still splitter should be hidden from end users of the library; the implementation for #56 has already introduced a splitter_port parameter (which only applies to video-port based output). Adding another one is going to lead to a horribly confusing API, and I can't think of a use-case for permitting the user to select the still splitter port manually at the moment.

@waveform80

This comment has been minimized.

Owner

waveform80 commented Mar 19, 2014

Urgh - shouldn't have spoken so soon. Turns out that while a splitter's output ports can be set to a different encoding to the input port, and raw output made to work successfully, as soon as an encoder is attached it fails to operate if the splitter's input and output formats don't match (probably some leaky abstraction).

Furthermore: the video splitter can operate with opaque encoding on its inputs and outputs successfully, but a splitter attached to the stills port only operates successfully with I420 (YUV 4:2:0) encoding - attempting to operate it with opaque encoding fails with no callbacks being issued despite capture apparently starting successfully.

So ... back to the drawing board: a splitter isn't the solution for the stills port, so we'll have to resort to reconfiguring the camera's still port when switching between raw and cooked still-port-based image capture (and add an exception for resizer usage which forces I420 mode as the resizer doesn't work with opaque encoding).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment