Lychee icon indicating copy to clipboard operation
Lychee copied to clipboard

[Enhancement] Size computation rather than command line I/O

Open ildyria opened this issue 2 years ago • 7 comments

After the merge of #1239 It will be possible to know all the sizes stored in the database. As a result, it would a great to update the code in DiskUsage to use those values rather than the I/O intensive computing currently done.

https://github.com/LycheeOrg/Lychee/blob/master/app/Metadata/DiskUsage.php#L139

ildyria avatar Mar 15 '22 21:03 ildyria

While I really like that idea, I just stumbled across a minor issue. We don't have the file size of the video part for live/motion pictures in the database.

nagmat84 avatar Apr 24 '22 12:04 nagmat84

@Chostakovitch ? :)

ildyria avatar Apr 29 '22 21:04 ildyria

Sorry, it's been too long and I cannot even see the following of the conversation on Gitter.

The simplest way is to do I/O for the video part because I suspect that most of users do not have their instance full of live photos (?). I think that storing the information in the DB would be too specific and introduce confusion.

Do you agree @nagmat84 ?

Chostakovitch avatar May 31 '22 08:05 Chostakovitch

See here: https://gitter.im/LycheeOrg/Lobby?at=6272553714df4e44f20bd365 and the subsequent messages.

The idea is to treat the video part as the 8th size variant and store it in size_variants. This has the following benefits:

  • Fast size computation: All we need is a simple SELECT SUM(filesize) FROM size_variants and we are done.
  • Smaller photos table: We remove columns from the photos table which are probably mostly null for the majority of users. This does not only save storage space (something which is rather negligible) but also makes hydration of photo models faster
  • Cleaner architecture: Parts of Lychee already consider the live photo as a size variant. For example, if one archives a photo one can select one of the usual size variants (thumb, ..., big) or live photo. Storing the live photo as a size variant in the DB does not only make this consistent, but also helps avoiding some ugly if-then-else-branches in the code which are currently necessary, because the live photo is always an edge case.

nagmat84 avatar May 31 '22 11:05 nagmat84

Thanks for the highlights, seems very elegant to me ! :) I'll probably work on it in a few weeks if it still has to be done.

Chostakovitch avatar May 31 '22 12:05 Chostakovitch

@Chostakovitch This is a follow-up of the recent discussion in the Gitter Lobby. This are some points of what needs to be done. Attention: The order is not in the correct temporal order, i.e. migration of values obviously needs to be done, before the old values are deleted.

  • Table size_variants
    • Additional column checksum of character varying(40) not null.
      • This column stores the checksum of every size variant and file
      • In order to keep the migration efficient, we should fill the column with the value 0000000000000000000000000000000000000000 for the existing size variants to indicate that the column is not set
    • Column type: Range ORIGINAL (0) through THUMB (6) need to be extended by LIVE (7).
  • Table photos
    • Column checksum: needs migration to size_variants.checksum for the original type, i.e. where size_variants.type == 0
    • Column live_photo_checksum: needs migration to size_variants.checksum for the live photo type, i.e. where size_variants.type == 7
    • Column live_photo_short_path: needs migration to size_variants.short_path for the live photo type, , i.e. where size_variants.type == 7
    • Column live_photo_content_id should be renamed to content_id (This has always bothering me, but now it its the time to do it in order to avoid confusion)
  • GoogleMotionPictureHandler (see app/Image/GoogleMotionPictureHandler)
    • Handles the extraction of the video stream using FFMpeg, see https://github.com/LycheeOrg/Lychee/blob/1a31f3c451907dcbea72ba39ac96ad6c1b390b3b/app/Image/GoogleMotionPictureHandler.php#L77-L83
    • Add a method to GoogleMotionPictureHandler::getDimension() which returns an ImageDimension similar to ImageHandler::getDimension()
  • AddStandaloneStrategy::do() needs tweaking here https://github.com/LycheeOrg/Lychee/blob/1a31f3c451907dcbea72ba39ac96ad6c1b390b3b/app/Actions/Photo/Strategies/AddStandaloneStrategy.php#L134-L149
    • Obviously, the live_photo_short_path needs not to be stored in photo, but in the correct size_variant
    • The line $videoTargetFile->write($tmpVideoFile->read()); should be $streamStat = $videoTargetFile->write($tmpVideoFile->read(), true); --> this gives you a StreamStat with a SHA1 checksum and the file size (hurrah, hurrah) to be stored in size_variant.checksum and size_variant.filesize resp.
  • AddVideoPartnerStrategy::do needs tweaking in a similar fashion
  • SizeVariants::create https://github.com/LycheeOrg/Lychee/blob/1a31f3c451907dcbea72ba39ac96ad6c1b390b3b/app/Models/Extensions/SizeVariants.php#L197 does not only need a parameter $filesize but also an additional $checksum or even better: use StreamStat to pass both values in a single parameter
  • SizeVariantDefaultFactory::createSizeVariantInternal https://github.com/LycheeOrg/Lychee/blob/1a31f3c451907dcbea72ba39ac96ad6c1b390b3b/app/Image/SizeVariantDefaultFactory.php#L185-L192 needs tweaking, save must be called with true as the 2nd parameter to get a StreamStat for the saved size variant, this StreamStat then needs to be passed to size_variants->create instead of only the filesize
  • Model Photos
    • Remove attribute checksum
    • Remove attribute live_photo_short_path
    • Remove attribute live_photo_checksum
    • Rename live_photo_content_id to content_id
  • Model SizeVariant: Add attribute checksum

This should be the most crucial steps. I haven't written up all the changes which are necessary to the rest of the code which assumes that the live photo is stored directly in the photo instead of a size variant. But this should rather be straight forward.

nagmat84 avatar Jul 29 '22 18:07 nagmat84

@nagmat84 Thanks, a lot, for taking the time to write this. Very helpful. I am unavailable for all august, and not much in september. If it is not urgent, and that nobody started to work on it by the time, I am still in!

Chostakovitch avatar Jul 31 '22 20:07 Chostakovitch