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.

Sign up for GitHub

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

Jump to bottom

Extvlcopt support matrix #244

Merged
merged 9 commits into from
Aug 14, 2019
Merged

Extvlcopt support matrix #244

merged 9 commits into from
Aug 14, 2019

Conversation

phunkyfish
Copy link
Member

v4.1.0

  • Support EXTVCOPT in m3u

@phunkyfish phunkyfish force-pushed the EXTVLCOPT-support-Matrix branch 3 times, most recently from b31ffc5 to 7dd7949 Compare June 30, 2019 09:09
@MilhouseVH
Copy link

This has been in test builds for almost a month (since #0630) and no reported issues - time to merge?

@phunkyfish
Copy link
Member Author

Yes, that would be great. There is also backport PR #245

@phunkyfish
Copy link
Member Author

@ksooo could you help with getting these PRs merged?

@MilhouseVH
Copy link

Ping...

@ksooo
Copy link
Member

ksooo commented Aug 9, 2019

could you help with getting these PRs merged?

Sure, if you could explain to me what this change is good for. The PR description tells me nothing. :-/

@phunkyfish
Copy link
Member Author

Completely fair.

We added code to the FFMPeg demux to support setting the program ID. The PR allows this to be set via M3u8 for simpleiptv.

@phunkyfish
Copy link
Member Author

I could now actually merge these myself once you are happy ;)

@ksooo
Copy link
Member

ksooo commented Aug 9, 2019

I spotted some strings that could be const. :-)

I could now actually merge these myself once you are happy ;)

Let's follow the official process also here. Once you have the formal approval of another guy with some knowledge you can merge it, like we've done/should also do in future with vuplus.

@phunkyfish
Copy link
Member Author

I spotted some strings that could be const. :-)

Will I ever learn...

Let's follow the official process also here. Once you have the formal approval of another guy with some knowledge you can merge it, like we've done/should also do in future with vuplus.

Sure, makes sense.

@phunkyfish phunkyfish force-pushed the EXTVLCOPT-support-Matrix branch 2 times, most recently from 6e07c83 to 5116dc2 Compare August 9, 2019 15:54
@phunkyfish
Copy link
Member Author

Jenkins looks to be stalled on this.

ksooo
ksooo reviewed Aug 10, 2019
View reviewed changes
ksooo
ksooo reviewed Aug 10, 2019
View reviewed changes
ksooo
ksooo reviewed Aug 10, 2019
View reviewed changes
ksooo
ksooo reviewed Aug 10, 2019
View reviewed changes
ksooo
ksooo reviewed Aug 10, 2019
View reviewed changes
@phunkyfish phunkyfish force-pushed the EXTVLCOPT-support-Matrix branch from 5116dc2 to c01b2fa Compare August 10, 2019 05:08
@phunkyfish phunkyfish force-pushed the EXTVLCOPT-support-Matrix branch from c01b2fa to 74e802c Compare August 10, 2019 05:17
ksooo
ksooo approved these changes Aug 10, 2019
View reviewed changes
Copy link
Member

@ksooo ksooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

@phunkyfish
Copy link
Member Author

jenkins build this please

@phunkyfish phunkyfish force-pushed the EXTVLCOPT-support-Matrix branch from 74e802c to b290e74 Compare August 10, 2019 06:13
@phunkyfish
Copy link
Member Author

I’m not sure why iOS is failing. It appears to build correctly.

@ksooo
Copy link
Member

ksooo commented Aug 10, 2019

I’m not sure why iOS is failing. It appears to build correctly.

Me neither. @Rechi do you have an explanation?

@Rechi
Copy link
Contributor

Rechi commented Aug 10, 2019

Because xbmc/xbmc#16039 changed the zip file name.

@Rechi
Copy link
Contributor

Rechi commented Aug 10, 2019

Change the ZIP name / path for iOS from <ADDON-ID>+darwin_embedded-<ARCH>/<ADDON-ID>-<VERSION>.zip back to <ADDON-ID>+ios-<ARCH>/<ADDON-ID>-<VERSION>.zip.

@ksooo
Copy link
Member

ksooo commented Aug 10, 2019

Change the ZIP name / path for iOS from +darwin_embedded-/-.zip back to +ios-/-.zip.

Sorry for my dumb questions: And where does this need to be fixed?

@Rechi
Copy link
Contributor

Rechi commented Aug 10, 2019

In the same repo as the PR, which broke it, https://github.com/xbmc/xbmc/.

@ksooo
Copy link
Member

ksooo commented Aug 10, 2019

In the same repo as the PR, which broke it, https://github.com/xbmc/xbmc/.

Over my head, sorry. Who could/must do this and where exactly?

@Rechi
Copy link
Contributor

Rechi commented Aug 10, 2019

I would wait for the PR author to have a look, but you can fix it too.

https://github.com/xbmc/xbmc/blob/43d3c7eb52c60a3f382b8e00d32abcdee1d5858e/cmake/scripts/common/AddonHelpers.cmake#L22-L24 defines the output name / path. So you can start from there.

@phunkyfish
Copy link
Member Author

This would be @fuzzard I guess. Related to tvOS. I think he’s away for a week though.

@phunkyfish
Copy link
Member Author

@SylvainCecchetto, is this something you could look at? You’re the only other cmake whiz I know, plus you have some knowledge of the PR. Totally understand if you are busy with your thesis.

@phunkyfish phunkyfish force-pushed the EXTVLCOPT-support-Matrix branch 3 times, most recently from 6dbb27d to 2188a2e Compare August 11, 2019 02:44
@phunkyfish phunkyfish force-pushed the EXTVLCOPT-support-Matrix branch from 2188a2e to d66adbb Compare August 11, 2019 03:16
@phunkyfish
Copy link
Member Author

phunkyfish commented Aug 11, 2019 •
edited
Loading

@Rechi

From his holiday fuzzard sent this on. Can't test it currently, for some reason I can't build iOS (local issues on laptop).

--- a/cmake/scripts/common/PrepareEnv.cmake    2019-07-23 19:32:19.000000000 +1000
+++ b/cmake/scripts/common/PrepareEnv.cmake    2019-08-11 08:43:59.000000000 +1000
@@ -46,9 +46,9 @@
   endif()
 elseif(CORE_SYSTEM_NAME STREQUAL darwin_embedded)
   if (CPU MATCHES armv7)
-    set(PLATFORM_TAG ${PLATFORM_TAG}-armv7)
+    set(PLATFORM_TAG ${CORE_PLATFORM_NAME_LC}-armv7)
   elseif (CPU MATCHES arm64)
-    set(PLATFORM_TAG ${PLATFORM_TAG}-aarch64)
+    set(PLATFORM_TAG ${CORE_PLATFORM_NAME_LC}-aarch64)
   else()
     message(FATAL_ERROR "Unsupported architecture")
   endif()

@fuzzard
Copy link

fuzzard commented Aug 11, 2019

Had time to do a quick test, CORE_PLATFORM_NAME_LC isnt available to prepareenv.cmake, however CORE_PLATFORM_NAME is. change that in the above diff, and i think you might be right.

@phunkyfish
Copy link
Member Author

PR for patch: xbmc/xbmc#16475

@phunkyfish phunkyfish force-pushed the EXTVLCOPT-support-Matrix branch from d66adbb to f811aa0 Compare August 11, 2019 07:45
@phunkyfish
Copy link
Member Author

We're back in action on Matrix at least. Thank @fuzzard and @Rechi

@ksooo had some time so added a bunch of refactoring, if you's like this moved to a new PR just say so.

@phunkyfish
Copy link
Member Author

phunkyfish commented Aug 11, 2019 •
edited
Loading

@MilhouseVH could you add this PR to the testbuilds?

EDIT: ignore me. Sure it’s already in there.

@phunkyfish phunkyfish force-pushed the EXTVLCOPT-support-Matrix branch from f811aa0 to c0e3233 Compare August 11, 2019 23:16
@phunkyfish phunkyfish force-pushed the EXTVLCOPT-support-Matrix branch from c0e3233 to 226f121 Compare August 12, 2019 01:08
@phunkyfish
Copy link
Member Author

Will merge this tomorrow assuming no reported issues with test builds.

@phunkyfish phunkyfish merged commit 67f0c0d into kodi-pvr:Matrix Aug 14, 2019
@phunkyfish phunkyfish deleted the EXTVLCOPT-support-Matrix branch August 14, 2019 18:30
@phunkyfish phunkyfish mentioned this pull request Aug 18, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ksooo ksooo ksooo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@phunkyfish @MilhouseVH @ksooo @Rechi @fuzzard