indi-gphoto improvements?

[knallio]

While looking at the indi-gphoto source in #173 and #176, I experimented with several aspects of the code which might lead to improvements. But since I have only hacked in the driver for a few days now I wanted to discuss these changes before I continue working on them.

• As far as I can see the call to raw2image() in is unecessary:
  

  indi-3rdparty/indi-gphoto/gphoto_readimage.cpp

  Lines 335 to 340 in 918e8c5

  	if ((ret = RawProcessor.raw2image()) != LIBRAW_SUCCESS)
  	{
  	DEBUGFDEVICE(device, INDI::Logger::DBG_ERROR, "Cannot convert %s : %s", filename, libraw_strerror(ret));
  	RawProcessor.recycle();
  	return -1;
  	}

  
  At least I can see no difference in the resulting image with and without it.

• With the aim to improve the situation with respect to indi_gphoto compatibility with libraw-2.0  #176 and changing frame dimensions I changed the way how the raw image is converted. Now the whole raw frame is decoded, and not only the image area as reported by libraw. This way the image cropping can be done by the user, so in principle one could get the old behavior (libraw-0.19), the new behavior (libraw-0.20), or the original canon behavior by defining the respective starting x and y values.
  On the one hand this leads to simpler code and more flexibilty, on the other hand it is harder for the user to know what parameters should be used. By setting "intuitive" values (starting values of 0, width and height as stated by the camera manufacturer), one would get black borders and not the whole image.

• In order to handle the shifting bayer patterns with different offsets I set the XBAYROFF and YBAYROFF tags if the respective margins are uneven. This would replace

  indi-3rdparty/indi-gphoto/gphoto_ccd.cpp

  Lines 1487 to 1492 in 918e8c5

  	// Align all boundaries to be even
  	// This should fix issues with subframed bayered images.
  	// subX -= subX % 2;
  	// subY -= subY % 2;
  	// subW -= subW % 2;
  	// subH -= subH % 2;

• When testing this I realized that kstars does not seem to respect those tags when debayering an image. The captured fits file is correctly rendered in siril. I did not look where the debayering takes place, if it is in kstars or indi.

• I removed lots of unused code, most of it for the dcraw related functions

All the changes can be seen in knallio@3c34350. Of course they are highly experimental...

Please let me know if any of this might be helpful/usable and if I should continue working on this.

[knro]

Regarding raw2image, I think at the time of writing this code the imgdata structure was not populated until this conversion was made. So you tested with unpack and that was sufficient to unpack everything including the memory image buffer?

Whatever changes made to the driver has to be consistent with how the driver is currently operating to avoid any regressions. We don't folks to throw away valuable frames due to incompatibilities in the produced frames (size or CFA pattern wise). Another issue is that this has to work across multiple manufactures (Canon, Nikon, Sony..etc).

[knallio]

So you tested with unpack and that was sufficient to unpack everything including the memory image buffer?

Yes. The documentation is a bit confusing, so I am not sure if it is true for all camera models, but in my case I did not need the raw2image function.

Whatever changes made to the driver has to be consistent with how the driver is currently operating to avoid any regressions. We don't folks to throw away valuable frames due to incompatibilities in the produced frames (size or CFA pattern wise).

Of course. That's why I wanted to discuss everything first. But in a way we already have the "regression" due to the libraw changes, i.e. calibration frames captured with libraw-0.19 will not work with light frames captured wit libraw-0.20

Another issue is that this has to work across multiple manufactures (Canon, Nikon, Sony..etc).

Agreed. However I can only test with Canon.

Some other aspects which could be changed in the driver:

• At the moment the raw file gets processed by libraw twice, first in gphoto_driver.cpp when extracting the temperature, working on the raw data in memory (as far as I understood):
  

  indi-3rdparty/indi-gphoto/gphoto_driver.cpp

  Lines 853 to 882 in 918e8c5

  	#if defined(LIBRAW_CAMERA_TEMPERATURE) && defined(LIBRAW_SENSOR_TEMPERATURE)
  	// Extract temperature(s) from gphoto image via libraw
  	const char *imgData;
  	unsigned long imgSize;
  	result = gp_file_get_data_and_size(gphoto->camerafile, &imgData, &imgSize);
  	if (result == GP_OK)
  	{
  	LibRaw lib_raw;
  	int rc = lib_raw.open_buffer((void *)imgData, imgSize);
  	if (rc != LIBRAW_SUCCESS)
  	{
  	DEBUGFDEVICE(device, INDI::Logger::DBG_DEBUG,
  	"Cannot decode (%s)", libraw_strerror(rc));
  	}
  	else
  	{
  	if (lib_raw.imgdata.other.SensorTemperature > -273.15f)
  	gphoto->last_sensor_temp = lib_raw.imgdata.other.SensorTemperature;
  	else if (lib_raw.imgdata.other.CameraTemperature > -273.15f)
  	gphoto->last_sensor_temp = lib_raw.imgdata.other.CameraTemperature;
  	}
  	lib_raw.recycle();
  	if (fd >= 0)
  	{
  	// The gphoto documentation says I don't need to do this,
  	// but reading the source of gp_file_get_data_and_size says otherwise. :(
  	free((void *)imgData);
  	imgData = nullptr;
  	}
  	}

and then again when processing the image data in gphoto_readimage.cpp, working on the temporary file on disk:

indi-3rdparty/indi-gphoto/gphoto_readimage.cpp

Lines 311 to 324 in 918e8c5

	int read_libraw(const char *filename, uint8_t **memptr, size_t *memsize, int *n_axis, int *w, int *h, int *bitsperpixel,
	char *bayer_pattern)
	{
	int ret = 0;
	// Creation of image processing object
	LibRaw RawProcessor;
	// Let us open the file
	if ((ret = RawProcessor.open_file(filename)) != LIBRAW_SUCCESS)
	{
	DEBUGFDEVICE(device, INDI::Logger::DBG_ERROR, "Cannot open %s: %s", filename, libraw_strerror(ret));
	RawProcessor.recycle();
	return -1;
	}

This led me to two questions:

• Could the temperature extraction be moved to the read_* functions in gphoto_readimage.cpp, to avoid processing the raw date twice?
• This would also solve the problem that the current code tries to get the temperature from the raw file even if the capture frame is not in raw format and throws an error in the log.
• As far as I could see all image reading functions could work either with raw data in memory provided by libgphoto2 and from a temporary file. In principle the temporary file is not needed, so everything could be changed to work in memory?

My main goals would be simpler code and hopefully some speed improvements. At the moment I take most of my images with a raspberry pi and the latency between capturing and showing the image can be annoying for tasks like polar alignment and focusing. The every second less would be an improvement.

In your opinion, should I continue working on this, or would it be better to avoid the risk of regressions and leave everything as it is?

[knro]

To follow up with this, the driver is working fine with libraw 20. Please reopen if you believe this is still an issue.