This repository has been archived by the owner on Feb 2, 2023. It is now read-only.
[ASVideoNode] Re-enabling HLS video constructed from URL #1748
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
ghost
added
the
|
@Xinchi: could you modify this PR to include [ASVideoNode] in front of the title? Thanks! |
maicki
changed the title
|
LGTM |
|
Tested in my app, LGTM |
|
@Xinchi - Please check over this first - #1600 There were issues with the previous method, as outlined above. One way to avoid the issues above may be to make the construction of the ASVideoNode class more explicit like so: I'm also finding that the current initialisation method is buggy. I find a lot of HLS videos that won't play. There are some examples here if you're interested: http://stackoverflow.com/questions/10104301/hls-streaming-video-url-need-for-testing |
…egate-superclass [ASVideoNode] Ensure that both ASVideoNode and ASNetworkImageNode delegate methods are called
…llDeprecate [ASCollectionView/ASTableView] Warn Users About Index Paths in willDisplayNode:/didEndDisplayingNode: Methods
…Parameter [ASTextNode / ASImageNode] Use structs for drawing parameter
[Unit Tests] Add Table View Data Thrash Testing
* [Carthage] Add cartfile, update example * [Carthage] fix travis build issue * [Carthage] fix cartfile for travis build error * capitalization fix
[ASLayoutSpec] Remove use of dictionary to hold children
…Layout [ASCollectionView, ASCellNode] Add support applyLayoutAttributes: on ASCellNode
…ion's context (facebookarchive#1759) * Don't relayout as a result of clearing a traitCollection's context I'm not completely sure this change is the best solution. Here is context: An ASEnvironmentTraitCollection has a pointer to an optional context that an ASVC is the owner of. When the ASVC is dealloc'ed, we go through all subnodes of the VC and clear out the context so that the struct isn't holding on to a garbage pointer. Setting the traitCollection on ASCollectionNode/ASTableNode causes the cells to relayout if the trait collection changed (this is a special case for these two nodes since their cells are not actually subnodes). Setting the context to nil registered as a trait collection change and was causing a layout even as we were dealloc'ing the VC. The logic I'm implementing here is: If the trait collection changed AND the displayContext did not, then we should relayout. If the trait collection changed AND the new displayContext is non-nil then we should layout In the case where the trait collection change was caused soley by the displayContext going from non-nil to nil, then we should NOT layout. ``` // At this point we know that the two traits collections are NOT equal for some reason BOOL needsLayout = (oldTraits.displayContext == currentTraits.displayContext) || currentTraits.displayContext != nil; ``` Is there a better place/safer way to do this? * removed extra setNeedsLayout call
…archive#1794) * [ASDataController] Add some assertions to clarify what queues things happen on * [ASCollectionDataController] Optimize willReloadData * [ASDataController] Minor optimizations, no functional changes * [ASDataController] Always reload data on _editingTransactionQueue * [ASDataController] Remove async data fetching option, deprecate callbacks * [ASDataController] Not mutable * [ASMultidimensionalArrayUtils] Use fast enumeration * Optimize ASMultidimensionalArrayUtils
…into ASVideoNode-overwrittenplaceholder
…sking dirty layout status
…ement [Followup] Small code improvements
[ASTextNode] use accessor for `pointSizeScaleFactors`
* [Infer] Fix 11 Infer errors/warnings * fix build error
…strainedSize. (facebookarchive#1939) * [ASDisplayNode] Short circuit measure calls that have a zero-area constrainedSize. // If the constrainedSize is completely zero-area, then there is no possibility for layout calculations to be successful. // This also avoids the issue of an inset being applied to 0, creating negative frame values. * [ASDisplayNode] Fix to shouldMeasure change. * One more fix.
…class naming collisions. (facebookarchive#1941)
…area constrainedSize. (facebookarchive#1939)" This reverts commit 3664ca6.
…d for conciseness and readability. (facebookarchive#1933) * [ASDisplayNode+AsyncDisplay.mm] Refactor display block creation method for conciseness and readability. * [ASDisplayNode+AsyncDisplay.mm] Some additional fixes / improvements that are required for the prior commit. * Fix one last spot of the merge with __instanceLock__.unlock();
…yout [ASCollectionView/ASTableView] Check for Zero-Size Correctly
|
Reopening this pull request as the bug has been observed again. @gazreese I totally agree with you on the duplicated .asset property. I will put up a PR that maps the ASVideoNode.asset to the internal asset object. |
…rGCD [ASDataController] Migrate Editing Queue to GCD for Performance/Debuggability
|
@Xinchi Sounds like a good idea to me. I'm using this functionality in my current project and don't mind testing for you if you like, just let me know when you've done the PR. |
Closed
Apple claims in the AVFoundationProgramming Guide that HLS videos can be constructed only through URL, but later with iOS 4.3 release notes it claimed to bring updates to how the HLS videos should be initialized, which works with asset too. I’ve tested with both, and it looks like initializing with asset is buggy.
|
Closing this PR, creating a new one : #1954 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Apple claims in the AVFoundationProgramming Guide that HLS videos can be constructed only through URL, but later with iOS 4.3 release notes it claimed to bring updates to how the HLS videos should be initialized, which works with asset too. I’ve tested with both, and it looks like initializing with asset is buggy.