df-playmedia-patch #34

Open
df wants to merge 14 commits from df/webif:df-playmedia-patch into master
df commented 4 months ago

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 4 months ago
df force-pushed df-playmedia-patch from 40a13a871a to e896ec5adc 3 months ago

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

The last hunk of webif/lib/system.class needs removing.
df commented 3 months ago
Poster

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

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 3 months ago
df added 1 commit 3 months ago
2ff0f14ae8 Fix DLNA playback on HDR
df added 1 commit 3 months ago
df added 1 commit 3 months ago
df added 1 commit 3 months ago
df force-pushed df-playmedia-patch from 01afb288bb to ed9df1796d 3 months ago
df added 1 commit 3 months ago

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 3 months ago
Poster

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 3 months ago
df added 1 commit 3 months ago
df commented 3 months ago
Poster

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 3 months ago
df force-pushed df-playmedia-patch from f6a3b999c4 to 44bb8f774d 3 months ago
df force-pushed df-playmedia-patch from 44bb8f774d to f0a1c0be1b 3 months ago
df added 1 commit 3 months ago

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 3 months ago
df force-pushed df-playmedia-patch from 36f40170e0 to 7964145677 3 months ago

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.
Owner

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.

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 3 months ago
Poster

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 2 months ago
df added 1 commit 2 months ago
df added 1 commit 2 months ago

/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 2 months ago
243dfdd595 Fix quoting and escaping of JS fragments

/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 2 months ago
df added 1 commit 2 months ago
df added 1 commit 2 months ago
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.