Project

General

Profile

Actions

Bug #1182

closed

If no media found, pipics loops fast and then locks up.

Added by Hammel 28 days ago. Updated 10 days ago.

Status:
Closed
Priority:
Immediate
Assignee:
Target version:
Start date:
19 Feb 2025
Due date:
% Done:

100%

Estimated time:
Severity:
01 - Critical

Description

The log shows this:

do_drawing[pipics.c:504] INFO Entered
do_drawing[pipics.c:515] ERROR imagePath is NULL.

It might not be locked up, but it appears to be. The system is not locked up as I can switch VTs and login.

Actions #1

Updated by Hammel 27 days ago

  • Severity changed from 03 - Medium to 01 - Critical
Actions #2

Updated by Hammel 24 days ago

  • Status changed from New to In Progress
  • % Done changed from 0 to 10

It's easy to see where this happens.
playImages() calls do_drawing() in an unbreakable loop, which returns 1 when there are no images to display. So this is where it's stuck.

It's not clear yet how to fix this, but the length of fileList in db.c, such as in getNextImage(), needs to be used to prevent spinning in a loop like this. If there are no images, don't do anything (or maybe display a warning message or something).

The fileList can be updated asynchronously by a watcher on the /media/usb path (or whatever dbtop is set to). But that's a different issue (RM #1183).

Actions #3

Updated by Hammel 23 days ago

I want to write a widget that displays an image. This can then be called to display the specified image, leaving selection of the image to the core.

To do this, I need to trace how pipics works.
  1. startKiosk is a timer thread that is called at boot to initialize running the playback.
  2. startKiosk calls playImages. playImages() loops to find the current image to draw, and then gets the next image. It then sets a timer to display the next image.
  3. do_drawing handles the actual drawing. It will return and error if the image can't be found.
  4. getNextImage (in db.c) sets the active image to the next image in the list of images, if any.
  5. pipicsNext kills the timer on the current image display, gets the next image and then displays it.
  6. pipicsPrev kills the timer on the current image display, gets the prev image and then displays it.
This is klunky, at best. Here is what I think would be better.
  1. Thread that identifies up to 100 images to display in the list of images(fileList in db.c). Thread updates list asynchronously if it finds more images or if images disappear from their paths (like a usb stick was removed).
  2. Thread that keeps up to 5 images in memory (or up to a given size of data). The middle image is the currently displayed image, unless there are <5 images.
  3. do_drawing() moves to a widget. Widget displays image at path or does nothing if path doesn't exist. It could also display an error message if images can't be found.
  4. getNextImage remains the same.
  5. pipicsNext updates active image, calls playImages.
  6. pipicsPrev updates active image, calls playImages.
  7. playImages stops timer, calls widget API to display current image, restarts timer.
  8. New timer function calls pipicsNext when it goes off.
  9. startKiosk calls playImages and starts new timer function.

The goal is to more clearly differentiate selecting the active image and actually displaying it.

Actions #4

Updated by Hammel 23 days ago

  • % Done changed from 10 to 50

Changes are implemented and ready to test on hardware.

Actions #5

Updated by Hammel 14 days ago

  • % Done changed from 50 to 70

Code is implemented and works much better. Exiting is immediate. But loading images is still slow. I need to add a GSList to cache a set of images, say 10 or 20, so that moving through the list appears faster. I can launch a thread to load images in the background, letting it exit when it has filled the cache. Then movement forward or back appends or prefixes images as needed. Append/prefix will be disabled until the cache is full.

One other item is that the default image is the Style.png for pipics. This is ugly. It should be a nice image (MIOT? Or maybe a nice puppy.) with text "Loading some images, please wait.".

The original problem of looping too fast with no images is essentially fixed, though only after I select a better default image.

Also, could use a way to identify orientation. Might be able to do that for PNG, JPEG, but not sure about others.
  1. https://libexif.github.io/
  2. https://github.com/winlibs/libjpeg
  3. http://www.libpng.org/pub/png/libpng.html
Actions #6

Updated by Hammel 13 days ago

Finding orientation from Exif data in an image file.
  1. Get an EXIF loader: ExifLoader *l = exif_loader_new();
  2. Load file into memory: exif_loader_write_file(l, char *path);
  3. Read data from loader: ExifData *ed = exif_loader_get_data(l);
  4. Get rid of the loader: exif_loader_unref(l);
  5. Get the Orientation tag: ExifEntry *entry = exif_data_get_entry (ed, EXIF_TAG_ORIENTATION);
  6. Get the value of the tag: char value2; exif_entry_get_value(entry, &value, 2);
  7. Based on value, rotate image appropriately.
  8. Possibly (might be handled by next step): exif_entry_unref(entry);
  9. Free ExifData: exif_data_unref(ed);

Exif data is in JPEG, but not standardized in PNG.

Actions #7

Updated by Hammel 13 days ago

I think orientation handling is already implemented with a call to gdk_pixbuf_apply_embedded_orientation() in the _paint() function of the widget.

I can verify that by extracting orientation information from some of the images in the sample data to see if they are being oriented correctly.

Actions #8

Updated by Hammel 13 days ago

Verified. Images are properly rotated with gdk_pixbuf_apply_embedded_orientation(). No need to integrate EXIF data reading into the widget.

Actions #9

Updated by Hammel 11 days ago

Last issue: caching pixbufs.

The first 10 pixbufs are loaded at boot time. This is okay except there is a delay before the default image is displayed. Loading filenames/pixbufs should be handled in a background task.

The background task/thread should:
1. Read all filenames as it does now.
2. Load only the first 10 pixbufs.
3. Schedule a background cache handler thread.

The GSList will need to be wrapped by a mutex for the initial background task, the cache handler and the getNext/getPrev functions.

This initial background task will be started where dbLoad() is currently called.
startKiosk() will sleep for 3 seconds after it loads the default image, then run as it normally does.
The secondary cache handler thread will keep at least 10 pixbufs. It looks at the offset of the activeFile and keeps at up to 5 behind and 5 ahead of it. If it appends one to the end, it unrefs one at the head. So the cache handler is a window into the full GSList of image files.
Every time the getNext or getPrev functions are called they must kick a semaphore to have the cache handler make it's updates.
Since getNext/getPrev can be faster than loading a pixbuf, the cache handler must load as many pixbufs as have been requested based on the window, the clear the semaphore.
It might be useful to limit the semaphore count to prevent the cache handler from being overloaded by user input.

Actions #10

Updated by Hammel 10 days ago

  • % Done changed from 70 to 90

Nearly done.

The creation and clearing of pixbufs needs to use a mutex on the GSList. I think I hit a point where two threads tried to update a pixbuf field at the same time.
I probably need to create a single function for clearing and one for creating the pixbufs, so they can be wrapped in the mutex.
In fact, the getting of the pixbuf needs to be wrapped too since I can't have the widget trying to use the pixbuf to update the display while someone else is trying to free it.

Actions #11

Updated by Hammel 10 days ago

  • Status changed from In Progress to Closed
  • % Done changed from 90 to 100

This is now completed. There is a new bug related to touch screens for pipics, but that's in RM #1184.

Code tested on both Media System with keyboard and on Kiosk with keyboard and touch screen.
Code committed and pushed.
Closing issue.

Actions

Also available in: Atom PDF