Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add dedicated layout for image mode #163

Merged
merged 4 commits into from
Nov 3, 2024
Merged

Conversation

Samillion
Copy link
Owner

Pinging @Commander07

Pinging @Xurdejl and @Keith94 as well.

Terribly sorry for the bother. Only if you can, could you help me test this please?

This is a restructure of #158

Now there are two layouts:

  • modern
  • modern-image

Handled with:

    -- load layout
    is_image() -- init check if it's an image file
    if state.is_image then
        layouts["modern-image"]()
    else
        layouts["modern"]()
    end

The is_image() function determines if the file is an image or not:

local function is_image()
    local current_track = mp.get_property_native("current-tracks/video")
    if current_track and current_track.image and not current_track.albumart 
       and mp.get_property_number("estimated-frame-count", 0) < 2 and audio_track_count == 0 then
        state.is_image = true
    else
        state.is_image = false
    end
end

Test script: /dev_imagemode_layout/modernz.lua

@Xurdejl
Copy link
Contributor

Xurdejl commented Nov 3, 2024

Love the new layout for images, looks so clean and works perfectly

@Samillion
Copy link
Owner Author

To do:

  • Refactor element visibility in layout not in osc init
  • Refactor title maxchars/clip in modern-image layout (base it on image width?)

@Commander07
Copy link
Contributor

I don't think loop and screenshot buttons should be included in the image layout. As images have no need to be looped and unless the image is non-local then you have no need for a screenshot button. And in the case of a non-local image you really should download the image not screenshot it.

@Samillion
Copy link
Owner Author

Fully agreed.

@Keith94
Copy link
Contributor

Keith94 commented Nov 3, 2024

Looks very nice. 👍

A tiny bit glitchy when advancing to next/previous image in a playlist (the full OSC pops up for a few frames)

@Samillion
Copy link
Owner Author

A tiny bit glitchy when advancing to next/previous image in a playlist (the full OSC pops up for a few frames)

Yeah, I've noticed that as well.

Once I get all the elements working correctly, and making sure that the regular layout wasn't affected by this change, I'll start addressing that issue.

@Keith94
Copy link
Contributor

Keith94 commented Nov 3, 2024

I want to apply keep-open=always for just images. Can I do that in auto profiles?

@Samillion
Copy link
Owner Author

Yup yup. I'll post an example auto-profile soon.

@Samillion
Copy link
Owner Author

OSC height should be decreased in modern-image, right? Since it holds less elements vertically.

@Keith94
Copy link
Contributor

Keith94 commented Nov 3, 2024

Ya, should be decreased.

@Keith94
Copy link
Contributor

Keith94 commented Nov 3, 2024

A tiny bit glitchy when advancing to next/previous image

Now it looks OK after applying ab1a762

@Xurdejl
Copy link
Contributor

Xurdejl commented Nov 3, 2024

I want to apply keep-open=always for just images. Can I do that in auto profiles?

This should do the work:

image-display-duration=inf

Edit: Initially suggested an auto profile, but since image-display-duration detects images defined as having only 1 video frame and no audio, the auto profile with was redundant.

@Keith94
Copy link
Contributor

Keith94 commented Nov 3, 2024

@Xurdejl Nice. That's even better. Somehow I missed image-display-duration in the docs lol. Thanks!

@Commander07
Copy link
Contributor

I want to apply keep-open=always for just images. Can I do that in auto profiles?

This should do the work

[image-files]
profile-cond=filename:match("%.avif$") or filename:match("%.bmp$") or filename:match("%.gif$") or filename:match("%.j2k$") or filename:match("%.jp2$") or filename:match("%.jpeg$") or filename:match("%.jpg$") or filename:match("%.jxl$") or filename:match("%.png$") or filename:match("%.svg$") or filename:match("%.tga$") or filename:match("%.tif$") or filename:match("%.tiff$") or filename:match("%.webp$")
image-display-duration=inf

There is no need for an auto profile for this as image-display-duration should only apply to images and can be include in your normal mpv.conf.

@Xurdejl
Copy link
Contributor

Xurdejl commented Nov 3, 2024

There is no need for an auto profile for this as image-display-duration should only apply to images and can be include in your normal mpv.conf.

Yeah, I realised shortly after posting it and edited the comment just when I got your notification, great timing haha

@Samillion Samillion merged commit eb7ad7e into main Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants