Skip to content
This repository has been archived by the owner on Nov 22, 2021. It is now read-only.

video's in het fotoboek #34

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open

Conversation

aykevl
Copy link
Contributor

@aykevl aykevl commented Jan 10, 2013

Fix voor issue #26. Mogelijkheid om video's in het fotoboek toe te voegen.
Dit zorgt ervoor dat video's netjes afgehandeld worden, op (vrijwel) dezelfde manier als afbeeldingen.

Extra vereisten:

  • Hiervoor is een wijziging in de tabel nodig, zie de wijzigingen in https://github.com/karpenoktem/knfotos/blob/master/SQL.
    • fa_photos.cache: extra set values '360p', '720p'
    • extra kolom in fa_photos: ``type enum('photo','video') NOT NULL default 'photo'
  • Een recente versie van ffmepg moet geïnstalleerd zijn. Zelf heb ik ffmpeg van source gecompileerd.
  • X-Sendfile moet ingeschakeld worden om video streamen te laten werken.

Dat was het volgens mij.

Wat het oplevert:

  • Video's in mp4 en webm, zodat alle moderne browsers via html5 video worden ondersteund.
  • Video's in lage (360p) en hoge (720p, HD) resolutie.

(potentiële) issues:

  • IE8 en lager worden niet ondersteund. Als alternatief is er een downloadlink beschikbaar. Eventueel kan een flash applet gebruikt worden, maar dat moet dan nog ingebouwd worden.
  • transcoden kan vrij lang duren (alhoewel, die servers zijn vrij vlot voor zover ik heb gezien, dus dat is niet echt een issue).
  • 720p is misschien wel erg groot. Deze kan te breed worden voor een gemiddeld beeldscherm.
  • Nog niks is te configureren. Dit wil ik nog toevoegen.
  • Verder heb ik nog niks gezien.

Wat vinden jullie er van?

@bwesterb
Copy link
Member

Ik kijk er nu doorheen, maar ik heb niet veel verstand van knfotos – Jille of Bart beheren knfotos.

SQL
`rotation` smallint(6) NOT NULL default '0',
`check_tags` tinyint(1) default '0',
`type` enum('photo','video') NOT NULL default 'photo',
PRIMARY KEY (`id`),
UNIQUE KEY `path` (`path`,`name`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8;
Copy link
Member

Choose a reason for hiding this comment

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

Wat is de SQL nodig voor de update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ik neem aan iets als dit:

alter table fa_photos add column type enum('photo','video');
alter table fa_photos modify column cached set('thumb','large','360p','720p','invalidated');

(niet getest! maar zou moeten werken)

Edit: de tweede regel is nu dit:

alter table fa_photos modify cached set('thumb','large','mp4_360p','webm_360p','mp4_720p','webm_720p','invalidated') NOT NULL;

Deze is wel getest (op de vorige tabelstructuur).
Edit2: extra fix.

@ghost ghost assigned Jille Jan 10, 2013
This array may be removed in the future.
Extracted the code and put it in a separate file 'media.php', resulting
in a *much* cleaner design.
Warning: when updating, these should also be applied to config.php.
Otherwise, knfotos won't work (it will display a warning).
Only tested in IE9 in IE7/8 mode. But I think it should work.
It uses an <object type="video/mp4"/> tag, so any plugin should work
(QuickTime is most likely).
Only tested in IE9 in IE7/8 mode. But I think it should work.
It uses an <object type="video/mp4"/> tag, so any plugin should work
(QuickTime is most likely).
@aykevl
Copy link
Contributor Author

aykevl commented Jan 12, 2013

Sorry voor de dubbele commit en de merge branch commits.

Opgelost:

  • Configuratie zit nu in config[.sample].php. Daarin zit het ffmpeg-pad en de codecs & resoluties om te ondersteunen.
  • Video zou nu onder zowat elke moderne browser moeten werken. Onder IE8 zou het nu ook moeten werken als het kan weergeven (dit is het geval als bijvoorbeeld QuickTime is geïnstalleerd, maar waarschijnlijk werkt het ook met andere mediaspelers als VLC. Standaard WMP speelt geen MP4).

    Nog een potentieel probleem:

    • Voor zover ik weet werken sessie cookies niet onder QuickTime in Safari. Dus filmpjes die alleen zichtbaar zijn voor leden zouden nog wel eens niet zichtbaar kunnen zijn in Safari op Mac OS X.

SQL
@@ -3,9 +3,10 @@ CREATE TABLE `fa_photos` (
`name` varchar(64) NOT NULL,
`path` varchar(255) NOT NULL,
`visibility` enum('deleted','lost','hidden','leden','world') NOT NULL default 'world',
`cached` set('thumb','large','invalidated') NOT NULL,
`cached` set('thumb','large','360p', '720p','invalidated') NOT NULL,
Copy link
Contributor

Choose a reason for hiding this comment

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

Kun je die options prefixen met video- ofzo? Zodat het duidelijk is dat het om video's gaat ipv hele grote foto's.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ja, je hebt gelijk. Zal ik dan ook een prefix/suffix maken voor de codec? Want dat hoort er denk ik eigenlijk ook in.

Copy link
Contributor

Choose a reason for hiding this comment

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

Converteer je elke video naar elke codec? Zo ja hoort dat er inderdaad ook in. Als je de codec prefixt kun je wat mij betreft video- weer weglaten.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ja. Dit heb ik net aangepast. Nu wordt mp4- of webm- als prefix gebruikt.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Net een foutje verbeterd. Blijkbaar accepteert (My)SQL geen - in een SET type, zonder er over te klagen. Ik heb het veranderd in een _ (mp4_360p etc.)

<source src="large.php?foto=<?= urlencode($foto) ?>&codec=<?= $codec ?>&res=<?= $res ?>" type="video/<?= $codec ?>" data-resolution="<?= $res ?>"/>
<?PHP } ?>
<?PHP } ?>
<p>Je browser ondersteund geen html5 video (gebruik <a href="http://windows.microsoft.com/nl-NL/internet-explorer/downloads/ie-9/worldwide-languages">IE9</a>+ of een recente versie van <a href="http://www.mozilla.org/nl/firefox/new/">Firefox</a>, <a href="https://www.google.com/intl/nl/chrome/browser/">Chrome</a>, <a href="http://www.apple.com/safari/">Safari</a> of <a href="http://www.opera.com/">Opera</a>). Toch willen we je niet in de steek laten en proberen we hieronder de video af te spelen.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

s/ondersteund/ondersteunt/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check. Zojuist verbeterd.

@Jille
Copy link
Contributor

Jille commented Jan 12, 2013

Ziet er goed uit. Je hebt wel zo veel veranderd dat het niet makkelijk meer helemaal te reviewen is dus zullen we dit gewoon moeten gaan testen.
Wel goed werk, de code kon inderdaad wel wat opschoning gebruiken.

@Jille
Copy link
Contributor

Jille commented Jan 12, 2013

Wat misschien nog wel een issue gaat worden nu we ook video's gaan transcoden. updatedb en updatecache worden elk uur (om xx:13) gestart. Video's transcoden zou best wel eens langer dan een uur kunnen duren en dan komen er twee processen die tegelijk bezig zijn en dat wordt alles dubbel gedaan en gaat het misschien zelfs stuk.

Dit kunnen we waarschijnlijk het beste oplossen door Francisca af te maken. Zij werd een daemon tegen wie je kon zeggen dat je de foto's wou updaten ipv elk uur een nieuwe starten.
We weten namelijk precies wanneer er nieuwe foto's beschikbaar zijn (die worden namelijk altijd via /fotos/admin/ in de foto-dir geplaatst.).
Die daemon zou dan kunnen voorkomen dat er twee processen tegelijk draaien én zorgt ervoor dat de foto's veel sneller beschikbaar zijn - en dat is wel een cool puntje naar de leden.

@bwesterb
Copy link
Member

Maar dit probleem kan ook direct opgelost worden met flock.

@aykevl
Copy link
Contributor Author

aykevl commented Jan 12, 2013

Ik heb nu net locking geïmplementeerd. Of Francisca waar je het over had er nu wel of niet komt, extra locking maakt het zaakje toch net wat stabieler lijkt me.
Als de database al gelockt is, zal een nieuw proces een fout geven en afsluiten. Dus zoals het nu gaat, zal de volgende update (in cron?) stoppen en het uur daarna plaatsvinden.

Locks may not always be released when PHP runs from a webserver when
there is an error. When run from the CLI (which is the case here), locks
are always removed when the process exits:
http://stackoverflow.com/questions/12651068/release-of-flock-in-case-of-errors
This because:
1. It makes it more clear that the formats are video resolutions, not
   photo resolutions.
2. It adds information about which video formats are transcoded.
@aykevl
Copy link
Contributor Author

aykevl commented Jan 13, 2013

Voor webm ondersteuning (dat beter is dan ogg) is op z'n minst versie 0.8 van ffmpeg nodig.
http://packages.debian.org/search?keywords=ffmpeg
Het staat in squeeze-backports, dus zou eenvoudig te installeren moeten zijn.
Bij deze, op aanvraag van Jille, een comment.

@aykevl
Copy link
Contributor Author

aykevl commented Jan 13, 2013

Jille, deze twee werken voor mij op khandhas (dit is uitgevoerd in /tmp):

avconv -strict experimental -loglevel warning -i MOV04724.MPG -strict experimental -ab 128k -b:v 500k -vf "scale=-1:360" -y MOV04724.mp4
ffmpeg -strict experimental -loglevel warning -i MOV04724.MPG -strict experimental -ab 128k -b:v 500k -vf "scale=-1:360" -y MOV04724.mp4

De eerste is letterlijk gekopieerd van toen ik een extra echo "$command\n" in updatecache.php invoerde. Dus op khandas moet het gewoon werken.

$ ffmpeg -version
ffmpeg version 0.8.3-6:0.8.3-1~bpo60+1, Copyright (c) 2000-2012 the Libav developers
  built on Jun 16 2012 10:32:51 with gcc 4.4.5
*** THIS PROGRAM IS DEPRECATED ***
This program is only provided for compatibility and will be removed in a future release. Please use avconv instead.
ffmpeg 0.8.3-6:0.8.3-1~bpo60+1
libavutil    51. 22. 1 / 51. 22. 1
libavcodec   53. 35. 0 / 53. 35. 0
libavformat  53. 21. 0 / 53. 21. 0
libavdevice  53.  2. 0 / 53.  2. 0
libavfilter   2. 15. 0 /  2. 15. 0
libswscale    2.  1. 0 /  2.  1. 0
libpostproc  52.  0. 0 / 52.  0. 0

config.php relevante regel:

$ffmpeg = 'avconv -strict experimental -loglevel warning';

Dus ik zou niet weten waarom het bij jouw niet werkt.

@aykevl
Copy link
Contributor Author

aykevl commented Feb 10, 2013

Het werkt nu op khandhas.
De configuratie die nodig is:

  • config.php moet een paar extra instellingen hebben - zie config.php.sample (alles onder 'video transcoding')
  • sql kolommen toevoegen, zie comments op SQL
  • zet alle huidige entries van fa_photos op type photo:
update fa_photos set type='photo';
  • X-Sendfile: zie http://redmine.lighttpd.net/projects/1/wiki/X-LIGHTTPD-send-file.
  • ffmpeg versie 0.8+ installeren (zit in de backports, zie khandhas). Versie 1.1+ zou betere resultaten geven (wat hogere kwaliteit), maar 0.8 doet alles dat nodig is.
  • ffmpeg locatie in config.php aanpassen indien nodig (misschien beter 'avconv' gebruiken om deprecation warnings te voorkomen).

Als het goed is, is dit alles.

@bwesterb
Copy link
Member

bwesterb commented Oct 6, 2013

@Jille ping

@Jille Jille removed their assignment Apr 7, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants