Bug 8377 - Show HTML5 video/ audio for media files in OPAC and staff client
Summary: Show HTML5 video/ audio for media files in OPAC and staff client
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: 3.12
Hardware: All All
: P5 - low new feature (vote)
Assignee: Mirko Tietgen
QA Contact: Jonathan Druart
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-06 10:54 UTC by Mirko Tietgen
Modified: 2014-05-26 21:04 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 8377 [ENH] Show HTML5 video/ audio in OPAC for media files in 856 (14.26 KB, patch)
2012-07-07 15:12 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 test data (MARC21) (1.68 KB, application/octet-stream)
2012-07-07 15:14 UTC, Mirko Tietgen
Details
Bug 8377 [ENH] Show HTML5 video/ audio in OPAC for media files in 856 (14.29 KB, patch)
2012-07-07 17:47 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio in OPAC for media files in 856 (14.29 KB, patch)
2012-07-11 20:07 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Remove previous version of sysprefs for this enhancement (314 bytes, text/x-sql)
2012-07-21 02:22 UTC, Mirko Tietgen
Details
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (19.83 KB, patch)
2012-07-21 02:40 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (20.02 KB, patch)
2012-07-21 02:52 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (19.21 KB, patch)
2012-07-21 02:59 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (19.11 KB, patch)
2012-07-22 04:51 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (2.04 KB, patch)
2012-07-22 22:58 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (19.10 KB, patch)
2012-07-27 09:38 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (19.10 KB, patch)
2012-08-10 16:16 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (19.10 KB, patch)
2012-08-10 16:18 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (19.14 KB, patch)
2012-08-10 20:07 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8377 [ENH] Followup: Move perl module to C4 (1.54 KB, patch)
2012-09-20 09:20 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (19.02 KB, patch)
2012-09-20 09:32 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (19.06 KB, patch)
2012-09-20 10:25 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (19.09 KB, patch)
2012-09-21 18:52 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (17.90 KB, patch)
2012-09-28 23:19 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377: Followup move style in a css file and do not pass template to a pm (7.25 KB, patch)
2012-11-09 14:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8377: Followup move style in a css file and do not pass template to a pm (7.23 KB, patch)
2012-11-22 08:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (17.87 KB, patch)
2012-11-22 12:26 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377: Followup move style in a css file and do not pass template to a pm (7.28 KB, patch)
2012-11-22 12:31 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (17.84 KB, patch)
2012-12-26 21:03 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8377: Followup move style in a css file and do not pass template to a pm (7.26 KB, patch)
2012-12-26 21:07 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client (17.90 KB, patch)
2012-12-27 09:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8377: Followup move style in a css file and do not pass template to a pm (7.32 KB, patch)
2012-12-27 09:03 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mirko Tietgen 2012-07-06 10:54:27 UTC
With HTML5 it is possible to integrate audio and video content into websites easily. This enhancement uses information from MARC field 856 to generate the appropriate HTML5 code to embed the file(s) in a tab in the opac-detail view, including a fullscreen option.

This enhancement introduces two sysprefs: one to enable the feature, one to specify file extensions to be recognizes as media files.

This patch supports the HTML5 <audio> and <video> element. 

Additionally it gives basic support for the <track> element. This element is not supported very well by recent browsers. It is possible to use javascript polyfills to support it, this may be an option to be introduced in a seperate enhancement.

The basic functions of this enhancement are MARC agnostic because MARC21 and UNIMARC both use field 856 for electronic resources. There are minor differences regarding subfields.

A test plan and sample MARC files will be added soon.
Comment 2 Mirko Tietgen 2012-07-07 15:12:02 UTC Comment hidden (obsolete)
Comment 3 Mirko Tietgen 2012-07-07 15:14:48 UTC
Created attachment 10685 [details]
Bug 8377 test data (MARC21)

Test data MARC21

Bug 8377 Test 01: Video, using subfields a, d, f
Bug 8377 Test 02: Video, using subfield u
Bug 8377 Test 03: Video, using subfields a, d, f, login, pass in l, k
Bug 8377 Test 04: Audio, using subfield u

Test plan

- Import attached test data. [I chose media typ 'bk' because it's probably present on most systems. It's not appropriate for the data but that doesn't matter.]
- Rebuild zebra -b -z
- Go to sysprefs->enhanced content->html5 media and set HTML5MediaEnabled to 'Show', save preferences.
- Search the OPAC for 'bug 8377'
- Get 4 results
- Open one result after the other. Observe working video (1-3) or audio (4) player on opac-detail page

Note that the video files don't have sound, that is not an error. Also note that the audio file is a trombone so turn down the volume before you try it :)

If you feel adventurous, go to sysprefs->enhanced content->html5 media and delete the entry for 'ogv' from HTML5MediaExtensions. Try the four biblios again and observe that only the audio example (4) shows a media player. You may have to empty your cache and restart Plack if you use that.

Some files contain subtitles to be displayed in a <track> element. These are not generally supported by browsers at the moment. You can see them in the source code of the opac-detail pages. Chrome/ Chromium can play them, but only if they are set as 'default'. There is no option to do that in this patch, so it won't work. Firefox can't play them yet. Opera can't play them yet. Internet Explorer 10 may be able to play them, but I don't have one. I included basic track support anyway and plan to continue working on that later. There is more on that matter in the Wiki. Please consider this patch working if you get working video or audio.
Comment 4 Mirko Tietgen 2012-07-07 15:32:12 UTC
This patch can be tested in a sandbox.
http://wiki.koha-community.org/wiki/Sandboxes
Comment 5 Mirko Tietgen 2012-07-07 17:47:28 UTC Comment hidden (obsolete)
Comment 6 Mirko Tietgen 2012-07-11 20:07:53 UTC Comment hidden (obsolete)
Comment 7 Mirko Tietgen 2012-07-11 20:09:44 UTC
Test plan

- Import attached test data. [I chose media typ 'bk' because it's probably present on most systems. It's not appropriate for the data but that doesn't matter.]
- Rebuild zebra -b -z
- Go to sysprefs->enhanced content->html5 media and set HTML5MediaEnabled to 'Show', save preferences.
- Search the OPAC for 'bug 8377'
- Get 4 results
- Open one result after the other. Observe working video (1-3) or audio (4) player on opac-detail page
Comment 8 Owen Leonard 2012-07-17 16:53:52 UTC
This looks like it's working well with the examples provided. The examples worked well in Firefox and Chrome. I also tested a record with an mp4 video attached in order to test Safari on Windows and Internet Explorer (9)--both working.

A couple of problems: The "HTML5 Media" tab is selected by default if HTML5 media is attached, which I don't think is correct. What is the reasoning behind changing the default tab in this case?

Also, I think labeling the tab "HTML5 Media" is not user-friendly at all. The "HTML5" aspect of it is useless information as far as a patron is concerned. Perhaps "Related media?"
Comment 9 Mirko Tietgen 2012-07-21 02:22:48 UTC
Created attachment 11056 [details]
Remove previous version of sysprefs for this enhancement

If you tested a patch for this bug before, you can use this file to delete the previous version of the sysprefs for this bug. They have changed in the new version - sorry.
Comment 10 Mirko Tietgen 2012-07-21 02:40:04 UTC Comment hidden (obsolete)
Comment 11 Mirko Tietgen 2012-07-21 02:52:07 UTC Comment hidden (obsolete)
Comment 12 Mirko Tietgen 2012-07-21 02:59:07 UTC Comment hidden (obsolete)
Comment 13 Mirko Tietgen 2012-07-22 04:51:48 UTC Comment hidden (obsolete)
Comment 14 Mirko Tietgen 2012-07-22 22:58:42 UTC Comment hidden (obsolete)
Comment 15 Mirko Tietgen 2012-07-27 09:38:02 UTC Comment hidden (obsolete)
Comment 16 Mirko Tietgen 2012-08-10 16:16:32 UTC Comment hidden (obsolete)
Comment 17 Mirko Tietgen 2012-08-10 16:18:39 UTC Comment hidden (obsolete)
Comment 18 Chris Cormack 2012-08-10 20:07:48 UTC Comment hidden (obsolete)
Comment 19 Paul Poulain 2012-09-15 05:52:33 UTC
QA comment:
This patch has one problem that must be solved before being pushed :
 the Koha:: namespace must contain OO code only, the Koha/HTML5Media.pm is not OO

I also think that this file (an OO version) should be in a subdirectory, but I've nothing to propose for now.

I've checked that the HTML5Media.pm passes perlcritic

2 options to earn passed QA :
 * rewritte HTML5Media.pm for an OO version
 * move it to C4/

failing QA for now.

note I haven't tested it, and it also not apply cleanly anymore:
CONFLICT (content): Merge conflict in opac/opac-detail.pl
Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/enhanced_content.pref
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/sysprefs.sql
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql
Auto-merging catalogue/detail.pl
Comment 20 Mirko Tietgen 2012-09-20 09:20:54 UTC Comment hidden (obsolete)
Comment 21 Mirko Tietgen 2012-09-20 09:32:24 UTC Comment hidden (obsolete)
Comment 22 Mirko Tietgen 2012-09-20 10:25:23 UTC Comment hidden (obsolete)
Comment 23 Mirko Tietgen 2012-09-20 10:35:22 UTC
Hi Paul,

thanks a lot for reviewing the patch. I have

- rebased to master
- moved HTML5Media.pm to C4
- changed references to C4::HTML5Media.pm in catalogue/detail.pl and opac/opac-detail.pl
- set up a sandbox to verify everything is OK
    http://catalogue.test7.biblibre.com/cgi-bin/koha/opac-search.pl?q=8377
    http://pro.test7.biblibre.com/cgi-bin/koha/catalogue/search.pl?q=8377
Comment 24 Mirko Tietgen 2012-09-21 18:52:17 UTC Comment hidden (obsolete)
Comment 25 Mirko Tietgen 2012-09-28 23:19:16 UTC Comment hidden (obsolete)
Comment 26 Jared Camins-Esakov 2012-10-06 04:30:50 UTC
Changing target version so this bug report will show up in the list of
3.12-targeted features and doesn't get lost in the shuffle during feature
freeze.
Comment 27 Jonathan Druart 2012-11-09 14:16:49 UTC Comment hidden (obsolete)
Comment 28 Jonathan Druart 2012-11-09 14:19:24 UTC
Hi Mirko,

Please take a look to my patch and test it. I change some code, I hope you are ok with it.
Comment 29 Mirko Tietgen 2012-11-21 22:08:15 UTC
Hi Jonathan,

with your followup I get 

Can't locate object method "field" via package "C4::Templates" at /home/mirko/koha/C4/HTML5Media.pm line 47.

in both OPAC and staff client.
Comment 30 Jonathan Druart 2012-11-22 08:28:03 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2012-11-22 08:28:55 UTC
(In reply to comment #29)
> Hi Jonathan,
> 
> with your followup I get 
> 
> Can't locate object method "field" via package "C4::Templates" at
> /home/mirko/koha/C4/HTML5Media.pm line 47.
> 
> in both OPAC and staff client.

Sorry Mirko, could you please test with this new patch please ?
Comment 32 Mirko Tietgen 2012-11-22 12:26:53 UTC Comment hidden (obsolete)
Comment 33 Mirko Tietgen 2012-11-22 12:31:35 UTC Comment hidden (obsolete)
Comment 34 Mirko Tietgen 2012-11-22 12:33:53 UTC
Rebased my patch, signed off the followup. Thanks Jonathan.
Comment 35 Marcel de Rooy 2012-12-20 08:48:48 UTC
Jonathan:
This patch was now first on the list for QA. 
It seems to me however that you forgot to sign off on the first patch of Mirko. Could you do that now?
Thanks.
Comment 36 Jonathan Druart 2012-12-20 13:38:03 UTC
(In reply to comment #35)
> Jonathan:
> This patch was now first on the list for QA. 
> It seems to me however that you forgot to sign off on the first patch of
> Mirko. Could you do that now?
> Thanks.

I never signed off this patch :)
Back to "Needs Signoff"
Comment 37 Marcel de Rooy 2012-12-20 14:31:44 UTC
Chris:
Looking at the history, I see that you did a signoff on this patch sometime ago. Could you please resign the current version?

Marcel
Comment 38 Chris Cormack 2012-12-26 21:03:40 UTC Comment hidden (obsolete)
Comment 39 Chris Cormack 2012-12-26 21:07:04 UTC Comment hidden (obsolete)
Comment 40 Jonathan Druart 2012-12-27 09:02:45 UTC
QA Comments:

The code is clean, nothing to add.
I proposed a followup for a little part of the code that required minor modifications.

Thanks for the test plan and the test data, it was very useful!

Marked as Passed QA.
Comment 41 Jonathan Druart 2012-12-27 09:03:03 UTC
Created attachment 14284 [details] [review]
Bug 8377 [ENH] Show HTML5 video/ audio for media files in OPAC and staff client

This enhancement uses information from MARC field 856 to generate the appropriate HTML5 code to embed am media player for the file(s) in a tab in the OPAC and staff client detail view. This patch supports the HTML5 <audio> and <video> element. Additionally it gives basic support for the <track> element. This element is not supported very well by recent browsers. Please consider the patch working when you get working video or audio.

Rebased to Master 22.11.2012

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 42 Jonathan Druart 2012-12-27 09:03:20 UTC
Created attachment 14285 [details] [review]
Bug 8377: Followup move style in a css file and do not pass template to a pm

1/ It's better to set style of the video marked in a css file.

2/ Replace some
  for my $i in (0..#$array)
with
  for my $element in (@array)

3/ the routine in a pm returns a hash not the $template variable

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 43 Jared Camins-Esakov 2012-12-27 14:29:00 UTC
This patch has been pushed to master.