Video files in album cause server error

[ctueck]

Detailed description of the problem

Albums with videos weren't shown, but caused an internal server error. The log reads:

[2021-08-03 09:33:23] production.ERROR: round(): Argument #1 ($num) must be of type int|float, string given {"exception":"[object] (TypeError(code: 0): round(): Argument #1 ($num) must be of type int|float, string given at /srv/www/.../app/Models/Extensions/PhotoCast.php:72)
[stacktrace]
#0 /srv/www/.../app/Models/Extensions/PhotoCast.php(72): round()
#1 /srv/www/.../app/Actions/Album/Photos.php(69): App\\Models\\Photo->toReturnArray()
#2 /srv/www/.../app/Actions/Album/Prepare.php(41): App\\Actions\\Album\\Photos->get()
#3 /srv/www/.../app/Http/Controllers/AlbumController.php(87): App\\Actions\\Album\\Prepare->do()
#4 /srv/www/.../vendor/laravel/framework/src/Illuminate/Routing/Controller.php(54): App\\Http\\Controllers\\AlbumController->get()
...

This points to the following line of code, which rounds the value of "focal" for videos:

Lychee/app/Models/Extensions/PhotoCast.php

Lines 71 to 72 in a385767

	// We need to format the framerate (stored as focal) -> max 2 decimal digits
	'focal' => (strpos($this->type, 'video') === 0) ? round($this->focal, 2) : $this->focal,

In my case, the value for focal was an empty string for all videos. (Basically all my photos/videos are synced from an iPhone to Nextcloud and then imported to Lychee, using cmdline sync operation, from the Nextcloud files.)

Not sure what it is that should be fixed - focal always containing a number, or the above code not relying on focal being a number.

For now, a simple work-around solved the issue for me:

update photos set focal = 0 where type like 'video/%';

(Caution for others: I had not videos with any value set in focal. If you have, you should probably add and focal = '' to the query.)

Steps to reproduce the issue

1. Open any album that contains a video where "focal" is not a number in the database.
2. Check log.

Output of the diagnostics

    Diagnostics
    -------
    Warning: Dropbox import not working. dropbox_key is empty.
    Error: A user is missing! Please create a user with id: "1"




    System Information
    --------------
    Lychee Version (git):            master (65383e4) - Data not in Cache
    DB Version:                      4.3.5
    
    composer install:                --no-dev
    APP_ENV:                         production
    APP_DEBUG:                       false
    
    System:                          Linux
    PHP Version:                     8
    Max uploaded file size:          30M
    Max post size:                   100M
    MySQL Version:                   10.3.29-MariaDB-0+deb10u1
    
    Imagick:                         1
    Imagick Active:                  1
    Imagick Version:                 1690
    GD Version:                      2.2.5




    Config Information
    --------------
    version:                         040305
    check_for_updates:               0
    sorting_Photos_col:              title
    sorting_Photos_order:            ASC
    sorting_Albums_col:              max_taken_at
    sorting_Albums_order:            ASC
    imagick:                         1
    skip_duplicates:                 0
    small_max_width:                 0
    small_max_height:                360
    medium_max_width:                1920
    medium_max_height:               1080
    lang:                            en
    layout:                          1
    image_overlay_type:              desc
    default_license:                 CC-BY-SA-4.0
    compression_quality:             90
    full_photo:                      1
    delete_imported:                 0
    Mod_Frame:                       1
    Mod_Frame_refresh:               30
    thumb_2x:                        1
    small_2x:                        1
    medium_2x:                       1
    landing_page_enable:             0
    landing_owner:                   Colin Tück
    landing_title:                   personal photo collection
    landing_subtitle:                everything & nothing
    landing_facebook:                https://www.facebook.com/colin.tueck/
    landing_flickr:                  
    landing_twitter:                 https://twitter.com/ctueck
    landing_instagram:               
    landing_youtube:                 
    landing_background:              
    site_title:                      Lychee v4
    site_copyright_enable:           1
    site_copyright_begin:            2000
    site_copyright_end:              2020
    additional_footer_text:          
    display_social_in_gallery:       1
    public_search:                   0
    SL_enable:                       0
    SL_for_admin:                    0
    public_recent:                   0
    recent_age:                      1
    public_starred:                  0
    downloadable:                    0
    photos_wraparound:               1
    map_display:                     1
    zip64:                           1
    map_display_public:              0
    map_provider:                    Wikimedia
    force_32bit_ids:                 0
    map_include_subalbums:           0
    update_check_every_days:         3
    has_exiftool:                    0
    share_button_visible:            1
    import_via_symlink:              0
    has_ffmpeg:                      0
    location_decoding:               0
    location_decoding_timeout:       30
    location_show:                   1
    location_show_public:            0
    rss_enable:                      0
    rss_recent_days:                 7
    rss_max_items:                   100
    prefer_available_xmp_metadata:   0
    editor_enabled:                  1
    lossless_optimization:           0
    swipe_tolerance_x:               150
    swipe_tolerance_y:               250
    local_takestamp_video_formats:   .avi|.mov
    log_max_num_line:                1000
    unlock_password_photos_with_url_param: 0
    nsfw_visible:                    1
    nsfw_blur:                       0
    nsfw_warning:                    0
    nsfw_warning_admin:              0
    map_display_direction:           1
    album_subtitle_type:             oldstyle
    upload_processing_limit:         4
    public_photos_hidden:            1
    new_photos_notification:         0

Browser and system

Firefox 90.0.2 on mac OS 11.4

[ildyria]

strpos($this->type, 'video') === 0 > actually this code should likely be changed for a is_video() 🤔

[nagmat84]

The problem occurs, because the focal and aperture attributes of a Photo model are exploited to store the duration and frame rate if the photo is actually a video. (I always forget which one maps to which one, but that is not important here.) While focal and aperture may be null in case of a photo, many parts of the code which deals with videos assumes that the duration and frame rate are always set. This whole code in this area is not very type safe.

This unfortunate design has evolved historically, because support for video has been added much later to Lychee and the way how videos are supported always feels a little bit "hack-ish".

I have already found many of those issues and corrected them, while I was heavily re-factoring the core part of Lychee. In particular, I added a lot of type hints and non-null checks into the code, because if those issues. I actually wonder, if the bug might be gone with PR #1055.

strpos($this->type, 'video') === 0 > actually this code should likely be changed for a is_video() thinking

@ildyria Totally correct. In the current master, there are still a lot of this kind of checks and corrected quite a few of them in my current PR.

[kamil4]

Yeah, that's definitely a bug, and a relatively new one at that, I think.

The code is not supposed to assume that the duration or frame rate of videos are known, as they are dependent on optional external binaries such as ffprobe.

All I can say is that it "used to work"...

[ildyria]

Fixed with #1113