dvdplayer: drop autocrop

[FernetMenta]

• autocrop is a rather outdated and useless feature. It analyzes a frame by comparing every pixel to a black level to estimate whether this is a black bar or not.
• The feature is buggy
• if it works then only for a very small number of systems which do sw decoding
• the implementation is a nightmare. it uses database entries as globals.

bye bye

[FernetMenta]

jenkins build this please

[Jalle19]

autocrap :-D I agree, almost everytime I've tried this feature it hasn't worked. It is a common feature on higher-end digital TV receivers though, mostly because there are still some channels that broadcast letterboxed pictures. That's about the only relevant use case though and those channels are rapidly disappearing.

[Voyager1]

disagree!!! My "Abyss" DVD has letterboxed 4:3 that can only be watched more or less full screen by using autocrop. OK to fix it, not ok to drop it.
Update: while it might be useless for current content, it's pretty useful for legacy content (SD DVDs that are pre-widescreen TV era) and that are SW decoded anyway.

[Jalle19]

@Voyager1 just change the zoom mode

[FernetMenta]

@Voyager1 feel free to stay with 14.0 for you ancient content or implement this in a proper way. the way it is, it has no chance to stay. no discussion.

[Voyager1]

the nice thing about autocrop is that it works "automatically". When you play DVDs with 4:3 menus that then go to a 16:9 letterboxed format, it zooms in automagically. This feature is there to stay IMO, so I would prefer you give it a chance to be properly refactored instead of just dropping it. I'll take a look at it in the next few days. Why the hurry anyway?

[FernetMenta]

this has been on my list for longer. there were a couple of reports in the forum. I am not fixing crap as you may remember from AE. For the majority of users this is a completely useless feature and it taints the design. You would need to implement this entirely new and you won't benefit from the old bits left in the code base.

[FernetMenta]

having this in video player is nonsense, you would need to move this into renderer.

[Voyager1]

makes sense. I would think that the algorithm part itself would be reusable to a large extent but agree with your suggestion to move it into the renderer. I don't hold you from going through with this, I'll need some time to look into a better way to implement it.

[fritsch]

The algo touched every row of video pixel by pixel (for loop) and comparing
the sum over it to certain thresholds that fell from heaven. This was very
very intrusive.

2015-01-08 23:07 GMT+01:00 Voyager1 notifications@github.com:

makes sense. I would think that the algorithm part itself would be
reusable to a large extent but agree with your suggestion to move it into
the renderer. I don't hold you from going through with this, I'll need some
time to look into a better way to implement it.

Reply to this email directly or view it on GitHub
#6154 (comment).

               Key-ID:     0x1A995A9B
               keyserver: pgp.mit.edu

Fingerprint: 4606 DA19 EC2E 9A0B 0157 C81B DA07 CF63 1A99 5A9B

[FernetMenta]

maybe you can use shaders. when doing yuv - rgb conversion the pixels have to touched anyway. you could pass the info on black bars back and use a 2nd rendering pass for cropping and scaling.

[FernetMenta]

jenkins build this please

[Memphiz]

bad influencing with build and merge of other job - jenkins build this please

[FernetMenta]

@Memphiz looks like all build errors are related to texturepacker

[FernetMenta]

jenkins build this please

[MartijnKaijser]

Can you also remove the related strings from our language file? Thx

[FernetMenta]

done

[Scondo]

1. It was bad and ugly, but it was way to start reworking. Now anyone who want to implement such feature must start from scratch.

2. Still some forgotten lines in header https://github.com/xbmc/xbmc/blob/master/xbmc/cores/dvdplayer/DVDPlayerVideo.h#L127

[fritsch]

@Scondo Feel free to pick it up, use the revert commit and improve it to a non pixel based approach.