search

osumoclement / script.black.bars.never

An addon to automatically remove black bars in KODI
GNU General Public License v3.0
12 stars 2 forks source link

Add option to toggle addon #3

Open TwilightMercy opened 1 year ago

TwilightMercy commented 1 year ago

Hi,

Great add-on and improvement to watching experience!

I have 2 questions:

  1. About android support - disabling hardware acceleration for the add-on to work will affect something?

Regardless the add-on - what’s the recommended setting for it?

And is there a progress with the fix for Android support?

  1. Is it possible to some how toggle on and off the black bar removal?

Currently the addon has no settings or windows, just enable/disable.

I was thinking about manually adding to Estuary skin player, a button that can toggle off and on the black bars removal, so it will be easier to choose how to watch the video during playback.

osumoclement commented 1 year ago

I will investigate this later

TwilightMercy commented 1 year ago

I will investigate this later

No problem, that’s a bonus for now 😃

What do you think about this? https://github.com/osumoclement/script.black.bars.never/issues/3#issuecomment-1601080518

osumoclement commented 1 year ago

Seems like a good way of handling such cases, will plan on implementation. In either case, the most important thing would be notifying the user that there are multiple aspect ratios, this way they can take manual actions if necessary

osumoclement commented 1 year ago

It would be good if you test the addon on multiple media and let me know how often it gets the aspect correct. Like does it work on 9 out of 10 media, that kind of statistic. And then for TV shows too, does it have less or more success when it comes to TV shows. I don't have internet on my playback device so its challenging to test this aspect.

TwilightMercy commented 1 year ago

It would be good if you test the addon on multiple media and let me know how often it gets the aspect correct. Like does it work on 9 out of 10 media, that kind of statistic. And then for TV shows too, does it have less or more success when it comes to TV shows. I don't have internet on my playback device so its challenging to test this aspect.

Sure. In addition to the notifications, you can log to kodi.log some information from the addon? How many aspects ratio found, their details, and log for each aspect ratio toggle. For easier debugging

osumoclement commented 1 year ago

I already log (sort of) but the most useful bit is the human experience - the visual aspect. Logs can show things that are very different to what is actually seen by the eyes . Addon could say it removed black bars but didn't, or partially removed 🤣

osumoclement commented 1 year ago

I see IMDbNumber is in KODI as seen here. So will work on using this as the primary way to identify media

osumoclement commented 1 year ago

https://alwinesch.github.io/group__python___info_tag_video.html

TwilightMercy commented 1 year ago

Good luck 💪🏻

osumoclement commented 1 year ago

Try now with the new code (not from release section) and see if IMDB ID is displayed at beginning of playback

TwilightMercy commented 1 year ago

Try now with the new code (not from release section) and see if IMDB ID is displayed at beginning of playback

Indeed!

(Anyway there should be a fallback if it doesn't exist I guess, but for 99% of the video addons - IMDB ID will exist.) image

osumoclement commented 1 year ago

Okay, I have uploaded new code which uses IMDB ID if exists and also detects if media has multiple aspect ratios and notifies user - In this case the addon does nothing and its up to the user to zoom manually. Later on I will implement the possible strategies discussed.

TwilightMercy commented 1 year ago

I tested new code on the movie "Ant-Man and the Wasp: Quantumania", which has 2 aspect ratios according to IMDb, results:

  1. When Android Workaround is DISABLED (normal method) -

Toggling the addon (I have the toggle button in my player skin) works without issues, and the notifications are the regular notifications that have always been.

If the movie has 2 aspect ratios, it zooms only at the current aspect ratio at the time of the toggle (screenshot) right? And If I'll toggle OFF and ON the addon again, but in the part of the movie with the 2nd aspect ratio, it will change according to that right?

  1. When Android Workaround is ENABLED -

I got the notification "Multiple aspect ratios detected, will do nothing", and after that an error: image

Error from log is:

error <general>: EXCEPTION Thrown (PythonToCppException) : -->Python callback/script returned the following error<--
                                                    - NOTE: IGNORING THIS CAN LEAD TO MEMORY LEAKS!
                                                   Error Type: <class 'UnboundLocalError'>
                                                   Error Contents: local variable 'aspectratio' referenced before assignment
                                                   Traceback (most recent call last):
                                                     File "C:\Users\twilight\AppData\Roaming\Kodi\addons\script.black.bars.never\addon.py", line 183, in <module>
                                                       p = Player()
                                                     File "C:\Users\twilight\AppData\Roaming\Kodi\addons\script.black.bars.never\addon.py", line 30, in __init__
                                                       self.abolishBlackBars()
                                                     File "C:\Users\twilight\AppData\Roaming\Kodi\addons\script.black.bars.never\addon.py", line 145, in abolishBlackBars
                                                       str(aspectratio) + ' ' + 'Player Aspect Ratio = ' + str(aspectratio2)
                                                   UnboundLocalError: local variable 'aspectratio' referenced before assignment
                                                   -->End of Python script error report<--
osumoclement commented 1 year ago
  1. Yes, it picks up the first aspect ratio and then when you'll have to toggle manually in scenes where it changes.
  2. Check now with new code
TwilightMercy commented 1 year ago
  1. Yes, it picks up the first aspect ratio and then when you'll have to toggle manually in scenes where it changes.
  2. Check now with new code

With android workaround enabled I played "Ant-Man and the Wasp: Quantumania",

The message Multiple aspect ratios detected, will do nothing still appeared, and after that the addon zoomed in 1.34 + the message: Wide screen was detected. Zoomed 1.34

The toggle function is still only between OFF and the 1.34 zoom. (tested for this movie with 2 aspect ratios)

osumoclement commented 1 year ago

Okay, I have removed the will do nothing part

TwilightMercy commented 1 year ago

Okay, I have removed the will do nothing part

Thanks, but can you still notify for multiple aspect ratios? so it will be known for the user.

And toggling between them/off state isn't implemented yet right?

osumoclement commented 1 year ago

Yes, it notifies and also does the zooming in both cases

Right now can only toggle on/off. Toggling between aspect ratios will be added much later

TwilightMercy commented 1 year ago

Yes, it notifies and also does the zooming in both cases

Great.

Right now can only toggle on/off. Toggling between aspect ratios will be added much later

I see. so for now - for Android workaround method - it will always use the first aspect ratio fetched from IMDb?

osumoclement commented 1 year ago

For Android, for now it uses the aspect ratio reported to KODI by the media. I am not exactly sure how the video would report multiple aspects, or if KODI even knows that videos could have multiple aspect ratios. It requires some investigation

TwilightMercy commented 1 year ago

For Android, for now it uses the aspect ratio reported to KODI by the media. I am not exactly sure how the video would report multiple aspects, or if KODI even knows that videos could have multiple aspect ratios. It requires some investigation

I thought there is no way in Android device to know the current aspect ratio, because of the screenshot limit. Or am I missing something? If it's being reported from KODI all along, couldn't it be used for all devices?

Edit: Oh I see, reported by the media. Maybe the media can have 2 aspect ratios in IMDB, but 1 reported to KODI. if that's the case - toggling between aspect ratios from IMDb API will be better I guess

osumoclement commented 1 year ago

The problem with relying on media reported aspect ratio is hard coded black bars. With these, the reported aspect ratio is misleading because the black bars are part of the video. That's why the addon has to check frames manually and also get original aspect ratio from IMDB

Yes, the media would probably report 1 aspect ratio, so toggling would be better

TwilightMercy commented 1 year ago

I see 👍🏻

I wish Android didn’t had that limitation and everything were much simpler.

osumoclement commented 1 year ago

It's quite unfortunate and I hope there will be a fix from either the GLES or KODI teams

TwilightMercy commented 1 year ago

I hope so.

For now, what’s planned next?

osumoclement commented 1 year ago

Next will be to add toggling between aspect ratios

test01203 commented 1 year ago

Next will be to add toggling between aspect ratios

Hey Thank you very much, for the most amazing add-on I tested it and it really does the job great I wanted to ask if there is a new, with multiple aspect ratios?

osumoclement commented 1 year ago

Not yet. Watch out for next release

test01203 commented 1 year ago

Not yet. Watch out for next release

Are you planning to release an update?

osumoclement commented 1 year ago

Yes, probably towards the end of September

osumoclement commented 1 year ago

New version released. For multiple aspect ratios, addon will notify and do nothing. This is because it is better for user to watch such media the way it is because if they change zoom, they won't know when the media aspect ratio changed later on in the video.

TwilightMercy commented 1 year ago

New version released. For multiple aspect ratios, addon will notify and do nothing. This is because it is better for user to watch such media the way it is because if they change zoom, they won't know when the media aspect ratio changed later on in the video.

It does nothing only for Android workaround? Or both?

osumoclement commented 1 year ago

Does nothing for both.

TwilightMercy commented 1 year ago

Does the toggle function (for button in skin) still exists?

RunScript(script.black.bars.never,toggle)

How it behaves?

osumoclement commented 1 year ago

Yes, behaves exactly as before. Though if there's multiple aspect ratios, toggling does nothing

TwilightMercy commented 1 year ago

Yes, behaves exactly as before. Though if there's multiple aspect ratios, toggling does nothing

I see. Why not at least toggle to the first aspect ratio? (probably the main one in the movie)

osumoclement commented 1 year ago

Because the addon can't know which is the first aspect ratio. The order in IMDB might not necessarily be the order they appear in the media. Plus if it did that anyway, user won't know when the aspect changes during playback so that they can unzoom. So basically, things are too messy in such situations, so addon just leaves everything entirely to the user