hummypkg/webif
5
3
Fork 4
Code Issues Pull Requests Releases 1 Activity

Play file in browser or with external helper application #34

Merged
prpr merged 14 commits from df/webif:df-playmedia-patch into master 2022-03-25 22:58:13 +00:00
Conversation 15 Commits 14 Files Changed 9 +214 -50
df commented 2021-02-16 02:31:30 +00:00
Owner
Copy Link

Replace the obsolete VLC player with the following scheme to play media in the browser or using an external player:

  • click on a media file in Browse Files;
  • the Media Details (pseudo-)dialog opens with Play and Close buttons;
  • if the file is a valid recording, the information is populated with recording data; the Play button closes the dialog and downloads a playlist file containing the file's URL (required because MPEG-TS is not supported as a container by any known browser);
  • if the file is a "general" media item, the information is populated with stream data from ffmpeg which is analysed to extract duration, container and video codec; the Play button closes the dialog and plays mp4, mkv and webm containers with h264, av1 or vp9 video in the browser's HTML5 video player, or otherwise downloads a playlist file as before;
  • in all cases the played URL is based on the hostname displayed in the browser's address bar (supposing the browser has anything so retro); this caters for NAT traversal, mDNS, etc

To do:

  • testing with various browsers;
  • testing with HDR;
  • maybe a setting to disable playing in the browser?

This patch should solve the problem raised in this forum thread and makes use of suggestions from forum user imanon.

Replace the obsolete VLC player with the following scheme to play media in the browser or using an external player: * click on a media file in Browse Files; * the Media Details (pseudo-)dialog opens with Play and Close buttons; * if the file is a valid recording, the information is populated with recording data; the Play button closes the dialog and downloads a playlist file containing the file's URL (required because MPEG-TS is not supported as a container by any known browser); * if the file is a "general" media item, the information is populated with stream data from `ffmpeg` which is analysed to extract duration, container and video codec; the Play button closes the dialog and plays mp4, mkv and webm containers with h264, av1 or vp9 video in the browser's HTML5 video player, or otherwise downloads a playlist file as before; * in all cases the played URL is based on the hostname displayed in the browser's address bar (supposing the browser has anything so retro); this caters for NAT traversal, mDNS, etc To do: * testing with various browsers; * testing with HDR; * maybe a setting to disable playing in the browser? This patch should solve the problem raised in [this forum thread](https://hummy.tv/forum/threads/is-this-firmware-abandoned-now-remote-streaming.10080/) and makes use of suggestions from forum user [imanon](https://hummy.tv/forum/members/imanon.13512/).
df added 11 commits 2021-02-16 02:31:31 +00:00 
Reviewed-on: df/webif#8
Also handle files with no "definition" as not ts.
Create a new `play.jim` here. The `/play` subtree is now obsolete and can be removed.
df force-pushed df-playmedia-patch from 40a13a871a to e896ec5adc 2021-03-13 19:21:16 +00:00 Compare
prpr commented 2021-03-14 01:05:58 +00:00
Owner
Copy Link

The last hunk of webif/lib/system.class needs removing.

The last hunk of webif/lib/system.class needs removing.
df commented 2021-03-14 11:56:30 +00:00
Author
Owner
Copy Link

Great minds, etc: I already prepared the attached patch (now deleted).

It appeared to apply to webif 1.4.9-3 using a real patch program:

patch -N -u -p 1 -d /mod < /mod/tmp/df-playmedia.diff

I don't have any confidence about the equivalent with Busybox patch:

(cd /mod; patch -N -p 1 ) < /mod/tmp/df-playmedia.diff

Also, you might think that rebasing the patch branch against the latest master would either eliminate any conflict or bring up a conflict resolution edit. When Linus named it git, I didn't realise he was an Alf Garnett fan.

Oh, and .patch isn't an allowed file type for the server and is secretly not attached. Admittedly not a complete secret if you know that there's a restriction, as its icon and tooltip try to distinguish the issue, but an actual error message, or just doing what you're told, would be better.

Update 2021-03-15 17:19
The previously attached zipped .diff finally correctly updated HDR as well as HD, but still didn't work with Busybox patch.

The same now applies to the default .diff file that Gitea generates, and you'll get the latest commits too. So:

  • download and install GNU patch linked from this thread into /mod/bin;
  • download the default .diff and apply it to a system with webif-1.4.9-3, using something like the first command above:
patch -N -u -p 1 -d /mod < /mod/tmp/34.diff
Great minds, etc: I already prepared the attached patch (now deleted). It appeared to apply to webif 1.4.9-3 using a [real patch program](https://hummy.tv/forum/threads/gnu-patch-for-cf.9513/post-136828): ``` patch -N -u -p 1 -d /mod < /mod/tmp/df-playmedia.diff ``` I don't have any confidence about the equivalent with Busybox patch: ``` (cd /mod; patch -N -p 1 ) < /mod/tmp/df-playmedia.diff ``` Also, you might think that rebasing the patch branch against the latest master would either eliminate any conflict or bring up a conflict resolution edit. When Linus named it git, I didn't realise he was an [Alf Garnett fan](https://www.theregister.com/2015/07/26/50_years_of_johnny_speight_comic_character_alf_garnett/). Oh, and `.patch` isn't an allowed file type for the server and is secretly not attached. Admittedly not a complete secret if you know that there's a restriction, as its icon and tooltip try to distinguish the issue, but an actual error message, or just doing what you're told, would be better. _Update_ 2021-03-15 17:19 The previously attached zipped `.diff` finally correctly updated HDR as well as HD, but still didn't work with Busybox patch. The same now applies to the [default `.diff` file that Gitea generates](https://git.hpkg.tv/hummypkg/webif/pulls/34.diff), and you'll get the latest commits too. So: * download and install GNU patch [linked from this thread](https://hummy.tv/forum/threads/gnu-patch-for-cf.9513/post-136828) into `/mod/bin`; * download the default `.diff` and apply it to a system with webif-1.4.9-3, using something like the first command above: ``` patch -N -u -p 1 -d /mod < /mod/tmp/34.diff
prpr commented 2021-03-15 10:03:03 +00:00
Owner
Copy Link

Still have "This pull request has changes conflicting with the target branch.
webif/lib/system.class"
Attaching patches isn't going to fix that, and proably just causes more confusion.

In any case, the patches are still broken:

"
humax /mnt/hd2/mod # patch -p1 <df-playmedia.patch
patching file webif/html/browse/download.jim
patching file webif/lib/system.class
patching file webif/lib/system.class
Hunk #2 FAILED at 312.
1 out of 2 hunks FAILED -- saving rejects to file webif/lib/system.class.rej
patching file webif/lib/setup
patching file webif/html/browse/index.jim
Hunk #2 succeeded at 139 (offset 3 lines).
patching file webif/html/browse/file.jim
patching file webif/html/browse/play.jim
patching file webif/html/browse/script.js
humax /mnt/hd2/mod # cat webif/lib/system.class.rej
--- webif/lib/system.class
+++ webif/lib/system.class
@@ -312,7 +312,7 @@ proc {system dlnaurl} {file {urlbase ""}} {
return $ret
}

-proc {system dlnahelper} {file {urlbase ""}} {
+proc {system dlnahelper} {file {urlbase "127.0.0.1"}} {
set dir /mnt/hd2/mod/.dlnahelper
require lock
"

After that, there is an unmatched brace at file.jim:264
I tried adding one in a likely looking place (274 or EOF) which kinda made things work.
The Media Details Info item just sits there say "Loading..." on a non-TS file.
The Play button didn't seem to get enabled either way.

Still have "This pull request has changes conflicting with the target branch. webif/lib/system.class" Attaching patches isn't going to fix that, and proably just causes more confusion. In any case, the patches are still broken: " humax /mnt/hd2/mod # patch -p1 <df-playmedia.patch patching file webif/html/browse/download.jim patching file webif/lib/system.class patching file webif/lib/system.class Hunk #2 FAILED at 312. 1 out of 2 hunks FAILED -- saving rejects to file webif/lib/system.class.rej patching file webif/lib/setup patching file webif/html/browse/index.jim Hunk #2 succeeded at 139 (offset 3 lines). patching file webif/html/browse/file.jim patching file webif/html/browse/play.jim patching file webif/html/browse/script.js humax /mnt/hd2/mod # cat webif/lib/system.class.rej --- webif/lib/system.class +++ webif/lib/system.class @@ -312,7 +312,7 @@ proc {system dlnaurl} {file {urlbase ""}} { return $ret } -proc {system dlnahelper} {file {urlbase ""}} { +proc {system dlnahelper} {file {urlbase "127.0.0.1"}} { set dir /mnt/hd2/mod/.dlnahelper require lock " After that, there is an unmatched brace at file.jim:264 I tried adding one in a likely looking place (274 or EOF) which kinda made things work. The Media Details Info item just sits there say "Loading..." on a non-TS file. The Play button didn't seem to get enabled either way.
df added 1 commit 2021-03-15 11:25:24 +00:00
df added 1 commit 2021-03-15 13:22:34 +00:00 
`urlbase` is the host name
df added 1 commit 2021-03-15 13:25:48 +00:00
df added 1 commit 2021-03-15 13:29:32 +00:00
df added 1 commit 2021-03-15 13:31:42 +00:00
df force-pushed df-playmedia-patch from 01afb288bb to ed9df1796d 2021-03-15 13:42:36 +00:00 Compare
df added 1 commit 2021-03-15 14:31:52 +00:00
prpr commented 2021-03-15 14:35:44 +00:00
Owner
Copy Link

Git: a system for making an unholy f---ing mess out of calm and order, causing much frustration in the process.

The source of the hassle appears to be this:
b0b1093eac
Hunk 2 has already been applied in hummypkg/master but somehow it isn't in this commit, so I guess this is what you need to rebase? Or maybe not, I dunno. Or just do whatever to make it go away and put the change in a separate PR.

The .patch file just seems to make more mess.
At least using the .diff file I only get this one conflict, which is easily dealt with.
Using the .patch file causes five.
I don't know when/why you would use .patch over .diff, or vice-versa.

Git: a system for making an unholy f---ing mess out of calm and order, causing much frustration in the process. The source of the hassle appears to be this: https://git.hpkg.tv/hummypkg/webif/commit/b0b1093eac75d55c27cf0f866ffe5592fbd0c5c4 Hunk 2 has already been applied in hummypkg/master but somehow it isn't in this commit, so I guess this is what you need to rebase? Or maybe not, I dunno. Or just do whatever to make it go away and put the change in a separate PR. The .patch file just seems to make more mess. At least using the .diff file I only get this one conflict, which is easily dealt with. Using the .patch file causes five. I don't know when/why you would use .patch over .diff, or vice-versa.
df commented 2021-03-15 15:18:14 +00:00
Author
Owner
Copy Link

There is an element of user error here (perhaps several).

Apparently .patch output is meant to be used with git am <patchfile>. For patch, one needs .diff.

Eliminating the duplicated change to system.class gives a .diff that applies to the latest beta using GNU patch. Busybox patch 1.20 fails to apply the change to webif/lib/setup which means you get to read the HTTP message containing the playlist instead of the playlist being downloaded and played. In either case the new file webif/html/browse/play.jim has to be made executable. Perhaps git am can do that from the create mode line in the .patch file.

And some bizarre typos and misunderstandings needed to be cleared up to get it working on HDRs as well as HDs.

I've updated the tweaked patch.

There is an element of user error here (perhaps several). Apparently `.patch` output is meant to be used with `git am <patchfile>`. For `patch`, one needs `.diff`. Eliminating the duplicated change to `system.class` gives a `.diff` that applies to the latest beta using GNU patch. Busybox patch 1.20 fails to apply the change to `webif/lib/setup` which means you get to read the HTTP message containing the playlist instead of the playlist being downloaded and played. In either case the new file `webif/html/browse/play.jim` has to be made executable. Perhaps `git am` can do that from the `create mode` line in the `.patch` file. And some bizarre typos and misunderstandings needed to be cleared up to get it working on HDRs as well as HDs. I've updated the tweaked patch.
df force-pushed df-playmedia-patch from fe57094a1a to 36b8b43a17 2021-03-15 15:50:33 +00:00 Compare
df added 1 commit 2021-03-15 15:51:16 +00:00
df commented 2021-03-15 15:53:06 +00:00
Author
Owner
Copy Link

And now unbundled the double update to eliminate the conflict.

And now unbundled the double update to eliminate the conflict.
df changed title from WIP: df-playmedia-patch to df-playmedia-patch 2021-03-15 15:53:24 +00:00
df force-pushed df-playmedia-patch from f6a3b999c4 to 44bb8f774d 2021-03-15 16:04:50 +00:00 Compare
df force-pushed df-playmedia-patch from 44bb8f774d to f0a1c0be1b 2021-03-15 16:08:52 +00:00 Compare
df added 1 commit 2021-03-15 16:37:16 +00:00
prpr commented 2021-03-15 17:06:12 +00:00
Owner
Copy Link

I've been using GNU Patch for a while (got fed up with lack of --dry-run on the stock one).
The create mode permissions seem to be 644 in the diff, which is not terribly useful as far as creating executable code goes, so I had to "chmod +x" it.
All working nicely now though.

I've been using GNU Patch for a while (got fed up with lack of --dry-run on the stock one). The create mode permissions seem to be 644 in the diff, which is not terribly useful as far as creating executable code goes, so I had to "chmod +x" it. All working nicely now though.
df added 1 commit 2021-03-15 17:13:34 +00:00
df force-pushed df-playmedia-patch from 36f40170e0 to 7964145677 2021-03-15 17:26:08 +00:00 Compare
prpr commented 2021-03-18 12:45:16 +00:00
Owner
Copy Link

Busybox patch 1.20 fails to apply the change to webif/lib/setup

It works OK for me.

> Busybox patch 1.20 fails to apply the change to `webif/lib/setup` It works OK for me.
af123 commented 2021-03-18 12:56:21 +00:00
Owner
Copy Link

Is this one ready to go into a beta? I can do the necessary re-basing etc.

git on the command line does handle conflicts and merges well in my experience, it's just a matter of going in and finding the conflict markers etc.

Is this one ready to go into a beta? I can do the necessary re-basing etc. `git` on the command line does handle conflicts and merges well in my experience, it's just a matter of going in and finding the conflict markers etc.
prpr commented 2021-03-18 13:30:14 +00:00
Owner
Copy Link

I think it is, but it's not my PR :-)
The /mod/webif/html/play tree needs removing too, as that's not in this.
It applies cleanly on 1.4.9-3 now, so no re-basing needed I would think.

I think it is, but it's not my PR :-) The /mod/webif/html/play tree needs removing too, as that's not in this. It applies cleanly on 1.4.9-3 now, so no re-basing needed I would think.
df commented 2021-03-18 15:24:00 +00:00
Author
Owner
Copy Link

What @prpr said.

I came to the conclusion that the problem area was a similar commit in the master and patch branches, where the patch commit repeated the change from master and added a further change. In rebasing you can only omit a commit or reorder/relog it but you can't modify the actual change. Once you get to merging, the conflict markers should come into play.

To-do item "test on HDR" has been achieved.

What @prpr said. I came to the conclusion that the problem area was a similar commit in the master and patch branches, where the patch commit repeated the change from master and added a further change. In rebasing you can only omit a commit or reorder/relog it but you can't modify the actual change. Once you get to merging, the conflict markers should come into play. To-do item "test on HDR" has been achieved.
df added 1 commit 2021-04-13 12:58:24 +00:00
df added 1 commit 2021-04-16 10:29:36 +00:00
df added 1 commit 2021-04-16 10:32:25 +00:00
prpr commented 2021-04-17 00:06:37 +00:00
Owner
Copy Link

/mod/webif/html/browse/file.jim:287: Error: missing quote
at file "/mod/webif/html/browse/file.jim", line 287

/mod/webif/html/browse/file.jim:287: Error: missing quote at file "/mod/webif/html/browse/file.jim", line 287
df added 1 commit 2021-04-17 01:55:37 +00:00 
NB jQuery $('#id') in a string looks like an [expr ...] to Jim!
prpr commented 2021-04-17 07:21:08 +00:00
Owner
Copy Link

/mod/webif/html/browse/file.jim:297: Error: unmatched "["
at file "/mod/webif/html/browse/file.jim", line 297
/mod/webif/lib/system.class:302: Error: invalid command name "http://aa.bb.cc.dd:9000/web/media/nnnnn.TS]"
at file "/mod/webif/lib/system.class", line 302

Line 300 looks wrong to me - spurious " }" on the end.

/mod/webif/html/browse/file.jim:297: Error: unmatched "[" at file "/mod/webif/html/browse/file.jim", line 297 /mod/webif/lib/system.class:302: Error: invalid command name "http://aa.bb.cc.dd:9000/web/media/nnnnn.TS]" at file "/mod/webif/lib/system.class", line 302 Line 300 looks wrong to me - spurious " }" on the end.
df added 1 commit 2021-04-17 12:09:31 +00:00
df added 1 commit 2021-04-17 13:42:56 +00:00
df added 1 commit 2021-04-17 14:13:10 +00:00
df changed title from df-playmedia-patch to Play file in browser or with external helper application 2021-10-07 19:54:30 +00:00
prpr merged commit 3dc3404327 into master 2022-03-25 22:58:13 +00:00
MymsMan commented 2022-03-25 23:13:17 +00:00
Contributor
Copy Link

This breaks the system dlnahelper function used by autodecrypt and chaseget see
Do not promote from Beta until fixed

Yes, that is the problem. The dlnahelper is deliberately using a link to the real ts file so that it doesn't conflict with the indexing of the real file by the humax so we must not use the normalized name.
I think it might work to create helper.ts as a hard link instead of symbolic link in system dlnahelper
Alternatively require the callers of system dlnaurl supply the normalized name when needed - are there many callers that don't
I could only find 3:

This breaks the system dlnahelper function used by autodecrypt and chaseget [see](https://hummy.tv/forum/threads/webif-1-4-9-7.10545/#post-161028) Do not promote from Beta until fixed Yes, that is the problem. The dlnahelper is deliberately using a link to the real ts file so that it doesn't conflict with the indexing of the real file by the humax so we must not use the normalized name. I think it might work to create helper.ts as a hard link instead of symbolic link in system dlnahelper Alternatively require the callers of system dlnaurl supply the normalized name when needed - are there many callers that don't I could only find 3:
prpr commented 2022-03-25 23:25:14 +00:00
Owner
Copy Link

I know. WiP. Will fix it all properly shortly.

I know. WiP. Will fix it all properly shortly.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: hummypkg/webif#34
No description provided.
Delete Branch "df/webif:df-playmedia-patch"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?