Improved video #1910
Merged
Improved video #1910
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marinofaggiana
force-pushed
the
improvedVideo
branch
3 times, most recently
from
8603267
to
74c50d7
Compare
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
marinofaggiana
force-pushed
the
improvedVideo
branch
from
92bcfd3
to
f3c42c0
Compare
Signed-off-by: marinofaggiana <ios@nextcloud.com>
|
@thisIsTheFoxe please approved this, thanks |
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
Signed-off-by: marinofaggiana <ios@nextcloud.com>
thisIsTheFoxe
suggested changes
Apr 11, 2022
There was a problem hiding this comment.
This breaks #1882 again..
UX: Subtitle button acts as a toggle. I think it might be clearer to do it similar to native iOS subtitles and have a separate "Off" subtitle..?
Notes:
- doesn't support embedded subtitles(?)
- subtitles don't support formatting
- subtitles need to be downloaded beforehand. Alternatively, could show an alert like: "Subtitle file(s) detected, download?"
| print("Play Subtitle at:\n\(url.path)") | ||
|
|
||
| let videoUrlTitle = self.metadata.fileName.alphanumeric.dropLast(3) | ||
| let subtitleUrlTitle = url.lastPathComponent.alphanumeric.dropLast(3) |
There was a problem hiding this comment.
Does this just remove the extension? Why do you need .alphanumeric? Why not just use deletingPathExtension()?
|
|
||
| alert.addAction(UIAlertAction(title: titleSubtitle, style: .default, handler: { [self] _ in | ||
|
|
||
| if NCUtilityFileSystem.shared.getFileSize(filePath: url.path) > 0 { |
There was a problem hiding this comment.
If subtitles can't be found they probably shouldn't be displayed in the first place.. I suggest to move this check outside before addAction
iOSClient/Viewer/NCViewerMedia/NCPlayer/NCSubtitle/NCSubtitlePlayer.swift
Show resolved
Hide resolved
thisIsTheFoxe
approved these changes
Apr 12, 2022
There was a problem hiding this comment.
looks good.
Just some minor optimisations I noticed:
- unused func
String.urlSafeBase64Decoded - unused l18n
_select_trace_,_video_must_download_, ... - unused vars
AssociatedKeys.FontKey,.ColorKey struct AssociatedKeysshould be anenum, so it can't be instantiatedNCSubtitlePlayer.swifthas a header ofAVPlayer+Extensions.swiftgetEncondingDataType(data:)can be refactored to use a loop. or.first(where:)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix:
Subtitles .SRT format:
moviefile
moviefile.lang.srt
example:
movie1.mov
movie1.eng.srt
movie1.ita.srt