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

MVC: fix subtitle scaling #17317

Merged
merged 1 commit into from Oct 4, 2022
Merged

Conversation

samnazarko
Copy link
Contributor

Description

This fixes the appearance of subtitles when playing back 3D MVC content and outputting it as Frame Packed.

Motivation and Context

Subtitles would not scale correctly before.
This should be backported to Leia.

How Has This Been Tested?

Tested on Raspberry Pi and Vero 4K +
(MMAL & AMCodec).

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

Signed-off-by: Sam Nazarko <email@samnazarko.co.uk>
@samnazarko
Copy link
Contributor Author

Unless there are any objections, I'd like to get this in.
CC @popcornmix

@afedchin
Copy link
Member

IMO this hack must be removed at all, I see no reason to support this tab/sbs subtitles.

@popcornmix
Copy link
Member

I wonder if this was a better solution: #8254

@phunkyfish
Copy link
Contributor

@samnazarko have you tried #8254?

@phunkyfish phunkyfish added the PR Cleanup: Ask Author PR appears to still be relevant and mergeable. Comment to author to check on status. label Mar 10, 2020
@phunkyfish phunkyfish added PR Cleanup: Recent Checked as part of PR cleanup. PR has been followed up on recently. and removed PR Cleanup: Ask Author PR appears to still be relevant and mergeable. Comment to author to check on status. labels Apr 16, 2020
@DaveTBlake DaveTBlake removed this from the Matrix 19.0-alpha 1 milestone Jul 15, 2020
@TheMontezuma
Copy link

I would really appreciate to have this bug fix integrated.
I use LibreElec with Raspberry Pi 3 and subtitles are often wrongly scaled/positioned when watching 3D MVC videos...

@TheMontezuma
Copy link

I tested this commit on RPI and it does not work

@DaveTBlake DaveTBlake added v20 Nexus and removed PR Cleanup: Recent Checked as part of PR cleanup. PR has been followed up on recently. v19 Matrix labels Feb 7, 2021
@DaveTBlake
Copy link
Member

@samnazarko no idea if this is still relevent. I have bumped to v20 in case you want to pursue, but please close (and set milestone to abandoned) if not. Thanks

@samnazarko
Copy link
Contributor Author

It's still relevant -- but the priority is low, as MVC 3D support was always supported with downstream out of tree patches.

This patch works for us on Vero 4K/4K+ (we can keep downstream for now); but a user reported it as not working on Pi.
When/if Pi's V4L2/GBM approach implements 3D MVC, we can revisit.

Cheers

@fuzzard
Copy link
Contributor

fuzzard commented Sep 19, 2022

@samnazarko another year on, whats the consensus on this?

@samnazarko
Copy link
Contributor Author

I will check internally. We still use this downstream, to my knowledge.

@samnazarko
Copy link
Contributor Author

It makes sense to merge this PR.

@garbear garbear added the Type: Fix non-breaking change which fixes an issue label Sep 20, 2022
@garbear garbear added this to the Nexus 20.0 Beta 1 milestone Sep 20, 2022
@samnazarko
Copy link
Contributor Author

I'm going to merge this.

@samnazarko samnazarko merged commit 05c9e1f into xbmc:master Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue v20 Nexus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants