Skip to content

addons: Topic Photos, freely licensed photos of your own topics as a card and a screensaver - #3283

Closed
Belseck wants to merge 4 commits into
Gathin23:mainfrom
Belseck:addons-topic-photos
Closed

Belseck wants to merge 4 commits into
Gathin23:mainfrom
Belseck:addons-topic-photos

Conversation

@Belseck

@Belseck Belseck commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

A new add-on, addons/topic-photos/: photos of the topics the owner picks or types ("mountains, ocean, city at night"), from Wikimedia Commons with no key, full screen in the app (its module card) and as a screensaver. Nothing under launcher/ changes.

Topic Photos: desert dunes under mountains, with the credit line at the bottom

  • Search at the screen's size. The resolution the device reports goes into each search (filew/fileh), so Commons returns no file smaller than the screen, and into the download (iiurlwidth), so it arrives at the screen's width. Landscape JPEGs only, at most three by one author per topic, about 30 kept on the TV and fetched again after three days or when the topics change.

  • The remote: Right shows another photo at random and Left goes back through the ones already shown, in order, Up shows the details (description, place, author and licence, date taken, size on the TV and the original's), Down or Back hide them, OK opens the settings.

  • Credit on every photo: author, Wikimedia Commons, licence and place, as CC BY and CC BY-SA ask. Place names come from the photo's coordinates through OpenStreetMap's reverse lookup, once per photo, in the TV's language.

  • Topics on a screen of their own: suggested topics grouped as holidays, seasons, and nature and more, ticked with OK, up to eight, and a field to type one's own; its suggestions complete it, so an existing topic is picked rather than spelt twice.

    The Topics screen: a field with an Add button, then suggested topics as tiles under Holidays, Seasons, and Nature and more, two of them ticked
  • Settings: topics, change photo every 10 s to 30 min, day-and-night photos (07:00-19:00 by the TV's clock, no location), and a shortcut to the TV's screensaver settings.

  • Permissions: INTERNET only (Commons and OpenStreetMap). No account, telemetry, ads or background service; both services get a User-Agent naming the app, as they ask. The README has the Permissions section and what is sent.

It grew out of the ideas discussion #3246. APK 28 KB, English and Spanish strings, JVM tests for the topic parsing, the eight-topic cap and the spelling of a typed topic, day and night, the size filter and the Commons width step.

Tested on: Chromecast with Google TV 4K (sabrina), Google TV 14: photos fetched for the default topics at 1920 x 1080, Left/Right, the details on Up and the settings on OK with keys sent over ADB, the place names, the launcher's prompt to show the card, and Topic Photos listed among the TV's screensavers. tests/test_addon*.py pass; lint has 0 errors.

Since the first review: the two races reported below are fixed. The carousel stops decoding, looking up and painting once released; a new photo list makes a decode queued for the old one stale; a place reply counts only while the same photo, by id, is on screen. The place is merged into the index under the same lock as its writes, reading the current index first, so it cannot put back an index a refresh replaced. Checked on the TV by opening and closing the viewer quickly five times: no crash. Left also used to step back one place in the list rather than to the photo shown before; a history of the photos shown now takes it back through them in order (JVM tests, and checked on the TV).

If this adds an add-on

  • addons/<name>/screenshot.jpg: the add-on on a real TV, 16:9 JPEG under 300 KB, shown in its row in addons/README.md

Related

One of five pull requests that came out of a Chromecast with Google TV 4K and the ideas discussion #3246:

This is where the fork's internet photos went: the launcher keeps no network, and the add-on carries the photos, the viewer and the screensaver.

🤖 Generated with Claude Code

…card and a screensaver

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

maintainer-bot review · this one needs a maintainer decision · Leans: close (60%)

Thank you for Topic Photos, especially the careful credit line on every photo and the searches matched to the screen's size. New network add-ons currently stay out of the repo unless the maintainer invited them, so this suggests keeping it in its own repo or fork with one row in SHOWCASE.md. If it was invited on an issue, please link that here.

Why it goes to a maintainer: confidence 0.60 is below 0.65.
The bot's own suggestion would be: your own fork, with a row in SHOWCASE.md.

Checks
  • Size: size/L (+2042 -0, 32 files)

This is a suggestion; a maintainer makes the final call. A maintainer can comment /bot review to run it again.

Reviewed by maintainer-bot · runs on 0G Private Computer

@SaviorOneZero

Copy link
Copy Markdown
Contributor

At 0a01b739b44be04b93608006c14ef725c17f9274, Carousel can accept work after its owner is released:

  1. show() posts its decoded-photo result to the main handler.
  2. ViewerActivity.onDestroy() / PhotoDream.onDetachedFromWindow() calls release(). This removes only tick, shuts down both executors, and leaves generation unchanged.
  3. The posted result passes the generation check and calls askPlace(). For a photo whose coordinates have not been resolved (placed == false), network.execute() throws RejectedExecutionException on the main thread.

A decode that finishes after release() has the same path. See release and decode callback → askPlace.

The same ownership gap exists on list replacement: display A at index 0, queue B's decode at index 1, then call setPhotos([A]). Its keep-current branch returns without invalidating B; B can subsequently paint and set index = 1 in the one-item list. Location callbacks also compare only the index, so A's late location reply can replace B's credit when index 0 is reused.

A focused fix is to mark release terminal, advance the decode generation on release and every list replacement, clear this handler's pending callbacks on release, and reject released/stale decode results. Check the current photo ID as well as index/liveness for location replies, preserving replies for a still-current same-ID photo across refresh.

I exercised the eight original lifecycle/selection methods verbatim with controlled Handler/Executor/View/Bitmap and decode/place boundaries: 7 of 13 cases fail before; all 13 pass with those guards, including queued and in-flight completion, list shrink/empty replacement, reused indices, pause/resume, and same-photo refresh controls. The six production Java files with the proposal also type-check against Android SDK 34. This is local JVM/source evidence, not Android rendering, real decoder/network or physical-TV validation.

@SaviorOneZero

SaviorOneZero commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

A separate disk-cache race at 0a01b739b44be04b93608006c14ef725c17f9274: Photos.place() reads the index outside writeIndex()'s class monitor. Synchronizing only the final write leaves this interleaving possible:

  1. A location lookup reads the old index containing A.
  2. The independent refresh executor prunes A's JPEG and publishes the new B index.
  3. The location lookup writes its old A index back, so the index references the deleted file and the newly cached B is unreachable through load().

Two Carousel instances' location executors can likewise read the same index and overwrite each other's location updates. Even if refresh keeps the same photo ID, replacing its entire JSON record with the lookup's older Photo can restore older credit metadata.

A small fix is a synchronized location-merge helper: acquire the same class monitor before reading the current index, update only place/placed on matching current IDs, then write while still holding that monitor; ignore photos no longer present. Keep the HTTP lookup outside the lock. This addresses these stale location commits; it does not make the entire fetch/prune/metadata pipeline transactional.

Local evidence: the original Photo, file/load/place/readIndex/writeIndex methods with real temporary index files and controlled HTTP/locale/log boundaries, ordering the existing monitor against refresh publication and two lookups: 3 of 7 cases fail before; all 7 pass with the helper. Controls cover normal lookup, removed-photo results, lookup failure and already-resolved photos. The fix also type-checks across all six production Java files together with the previous Carousel proposal against SDK 34 (inert resource constants). No live HTTP, JPEG decode, Android UI or physical-TV test ran.

Correction: cache filenames are fixed as id + ".jpg"; the retained-ID case concerns stale credit metadata.

Belseck and others added 2 commits November 4, 2026 19:19
The Topics row opens a full screen: suggested topics by group (holidays,
seasons, nature and more) ticked with OK, up to eight, and a field to type
one's own, which the suggestions complete so an existing topic is not
spelt twice. Replaces the comma-separated text dialog.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The carousel stops decoding, looking up and painting once released, and a
new photo list makes a decode queued for the old one stale. A place reply
is kept only while the same photo, by id, is on screen. The looked-up place
is merged into the index under the same lock as its writes, reading the
current index first, so it cannot put back an index a refresh replaced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot removed the bot-leans:close Held for the maintainer; the review leans close label Nov 5, 2026
@Belseck

Belseck commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for both reports and the careful interleavings. Fixed in 19c5e6c:

  • Carousel.release() now marks the carousel released, advances the decode generation and clears its handler; show(), decode results and askPlace() do nothing once released, so no work reaches a shut-down executor.
  • setPhotos() advances the generation on every list replacement, so a decode queued for the old list cannot paint into the new one.
  • A place reply is applied only while the same photo, by id, is on screen, and it is copied onto the list's current copy of that photo.
  • Photos.mergePlace() takes the class monitor before reading the index, changes only place/placed on the matching id, and writes while still holding it; a photo no longer in the index is left out. The HTTP lookup stays outside the lock.

Checked on the TV by opening and closing the viewer quickly five times, with no crash in the log.

Left used to step back one place in the list, which was not the photo
shown before, since Right picks at random. A history of the photos shown
now lets Left go back through them in order, and Right and the timer go
forward through them again before picking a new one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the bot-leans:close Held for the maintainer; the review leans close label Nov 5, 2026
@SaviorOneZero

Copy link
Copy Markdown
Contributor

Rechecked f539912226dadd0590d79cb5a318f6c94371f03d: the previous 13 controlled Carousel cases and 7 index cases pass with your fixes. There is a separate refresh handoff problem in Photos.refresh(): RUNNING drops the new caller's completion entirely.

With an empty cache:

  1. Viewer A starts a fetch, then closes before it finishes.
  2. Viewer B opens, loads the still-empty cache and calls refresh(). The failed compareAndSet returns without retaining B's callback.
  3. The fetch saves photos, but its only callback belongs to A. ViewerActivity ignores that destroyed owner. B never receives setPhotos() and stays on the finding screen until another resume. A topics change during the first fetch can likewise lose its refresh request.

A focused option is to queue stale requests on the existing single worker, recheck the requested cache inside each queued job, then either reuse it and post that caller's completion or fetch the differing terms. This preserves completion without a duplicate successful fetch for matching terms. No polling or additional executor is needed; queued requests retain their callbacks until processed.

A local one-file prototype passes 9 controlled cases; 6 fail before: two subscribers, close/reopen, changed topics, false/throwing fetches, null callback, fresh cache and main-handler delivery. All eight production Java files type-check against SDK 34 with the proposal. Tests use actual refresh/cache methods, temporary index files and controlled worker/handler/fetch boundaries; no live HTTP, JPEG decode, Android rendering or physical-TV test ran.

@SaviorOneZero

Copy link
Copy Markdown
Contributor

A separate failed-decode path at f539912226dadd0590d79cb5a318f6c94371f03d: show() can display one photo with another photo's credit/details.

When decode() returns null, the bitmap replacement is skipped, but index = chosen, the credit update, fillInfo() and askPlace(p) still run. If A is visible and B cannot be decoded, A stays on screen while the credit and expanded details identify B; B can also start a location lookup despite never being displayed. decode() explicitly returns null for invalid image dimensions, so this is an expected failure boundary.

A small fix is to put the index, credit, details and place-lookup updates inside the existing if (b != null) block, together with the bitmap swap. Keep schedule() outside it so the existing active timer continues. This preserves the previous frame and its metadata without adding a retry policy.

Local validation uses the actual setPhotos/step/show/lifecycle methods and PhotoHistory, with controlled decoder, Handler, executor and view boundaries: 4/8 new cases fail before; all 8 pass after, plus all 13 previous ownership controls. All eight production Java files with the one-file proposal type-check against SDK 34. No actual JPEG decode, Android rendering, HTTP or physical-TV test ran. This does not fix the separate refresh handoff or history being advanced before a frame wins.

@Gathin23

Gathin23 commented Nov 5, 2026

Copy link
Copy Markdown
Owner

@Belseck thank you for Topic Photos, especially the credit line on every photo and the searches sized to the screen. New network add-ons stay out of the repo unless the maintainer invited them, so this fits best as its own repo plus one SHOWCASE.md row with a screenshot link, which is very welcome. Before you publish it, the refresh() callback drop and the failed-decode credit mismatch SaviorOneZero found are worth fixing.

@Gathin23 Gathin23 closed this Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot-leans:close Held for the maintainer; the review leans close needs-maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants