Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.9
    • Fix Version/s: 2.0.3
    • Component/s: Filters
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      16557

      Description

      I'm maintaining Moodle package in Debian and I would have a query I would like to discuss with fellow developers: mediaplugin filer contains a flash file without a source code. I would like to suggest replacing it with some other Open Source player, e.g. flowplayer (related to http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=591201#23 ).

        Issue Links

          Activity

          Hide
          Petr Škoda added a comment -

          +1 to fix this in 2.0, but we need to act asap...

          Show
          Petr Škoda added a comment - +1 to fix this in 2.0, but we need to act asap...
          Hide
          Helen Foster added a comment -

          Tomasz, thanks for your suggestion. Just adding a link to the discussion about this issue: http://moodle.org/mod/forum/discuss.php?d=156608

          Show
          Helen Foster added a comment - Tomasz, thanks for your suggestion. Just adding a link to the discussion about this issue: http://moodle.org/mod/forum/discuss.php?d=156608
          Hide
          Martin Dougiamas added a comment -

          Rosie I need you to try replacing the .swf in filter/mediaplugins with flowplayer and set up a demo for me

          Show
          Martin Dougiamas added a comment - Rosie I need you to try replacing the .swf in filter/mediaplugins with flowplayer and set up a demo for me
          Hide
          Tomasz Muras added a comment -

          We need to keep in mind that THEME variable "filter_mediaplugin_colors" may stop working when we change the player.
          Also, in some places (like in the resource in Moodle 1.9), the player is called directly - we should make sure that it will still work or find & change that code as well.

          We can actually have someone here in Enovation to work on it asap and deliver new version of mediaplugin filter. I understand that we should hurry up to get that included in Moodle 2?

          Show
          Tomasz Muras added a comment - We need to keep in mind that THEME variable "filter_mediaplugin_colors" may stop working when we change the player. Also, in some places (like in the resource in Moodle 1.9), the player is called directly - we should make sure that it will still work or find & change that code as well. We can actually have someone here in Enovation to work on it asap and deliver new version of mediaplugin filter. I understand that we should hurry up to get that included in Moodle 2?
          Hide
          Joseph Rézeau added a comment -

          A brand-new moodle mp3 flash player is very much needed for 2.0. The player that is available in 1.9 is quite old and obsolete. See this discussion: http://moodle.org/mod/forum/discuss.php?d=155245
          Joseph

          Show
          Joseph Rézeau added a comment - A brand-new moodle mp3 flash player is very much needed for 2.0. The player that is available in 1.9 is quite old and obsolete. See this discussion: http://moodle.org/mod/forum/discuss.php?d=155245 Joseph
          Hide
          Tomasz Muras added a comment -

          Flowplayer could be used for both: playing video and as a mp3 player.

          Show
          Tomasz Muras added a comment - Flowplayer could be used for both: playing video and as a mp3 player.
          Hide
          Martin Dougiamas added a comment -

          Tomasz, if you are able to implement Flowplayer as a replacement I'd love to see a patch here!

          Show
          Martin Dougiamas added a comment - Tomasz, if you are able to implement Flowplayer as a replacement I'd love to see a patch here!
          Hide
          Martin Dougiamas added a comment -

          And yes, it's quite urgent if it is to get into 2.0

          Show
          Martin Dougiamas added a comment - And yes, it's quite urgent if it is to get into 2.0
          Hide
          Rossiani Wijaya added a comment -

          Hi Martin,
          I made changes to mediaplugins to use flowplayer (for flash and mp3 files). I can show you the changes on Thursday.

          Show
          Rossiani Wijaya added a comment - Hi Martin, I made changes to mediaplugins to use flowplayer (for flash and mp3 files). I can show you the changes on Thursday.
          Hide
          Tomasz Muras added a comment -

          I was afraid we will do a duplicate work - I did it for flash and .mp3 as well.
          I've set it up on http://demo.enovation.ie/moodle3 - login as a guest.
          There are still few problems there - I have noticed that the regular expressions that match the file patterns are not correct - they will not work sometimes (e.g. if you embed .flv first and .mp3 later on).
          Also, there will be a need to update the code in few other places, where usage of flv player is hardcoded.

          I will do that all and attach the patches - unless Rossiani has done a lot of work on it already - please let me know.

          Show
          Tomasz Muras added a comment - I was afraid we will do a duplicate work - I did it for flash and .mp3 as well. I've set it up on http://demo.enovation.ie/moodle3 - login as a guest. There are still few problems there - I have noticed that the regular expressions that match the file patterns are not correct - they will not work sometimes (e.g. if you embed .flv first and .mp3 later on). Also, there will be a need to update the code in few other places, where usage of flv player is hardcoded. I will do that all and attach the patches - unless Rossiani has done a lot of work on it already - please let me know.
          Hide
          Rossiani Wijaya added a comment -

          Hi tomasz,

          I will definitely take a look your demo site tomorrow and let you know if I find any differences.

          This is what I did on my local 2.0 vesion: replaced the .swf file in filter/mediaplugins and lib/mp3media with flowplayer and update the embedded code. I will continue working on this issue when I'm back to the office on Thursday.

          Show
          Rossiani Wijaya added a comment - Hi tomasz, I will definitely take a look your demo site tomorrow and let you know if I find any differences. This is what I did on my local 2.0 vesion: replaced the .swf file in filter/mediaplugins and lib/mp3media with flowplayer and update the embedded code. I will continue working on this issue when I'm back to the office on Thursday.
          Hide
          Tomasz Muras added a comment -

          Recent changes in Moodle 2 media filter embedding has caused the same problem as we've had before - see MDL-8919 .

          Show
          Tomasz Muras added a comment - Recent changes in Moodle 2 media filter embedding has caused the same problem as we've had before - see MDL-8919 .
          Hide
          Tomasz Muras added a comment -

          The problem with the filter not working for more than one media file is caused by create_ufo_inline - that embeds ufo.js each time it's called. If ufo.js is included on the page only once, our problem is solved. Now, the note in create_ufo_inline suggests that it's not possible (from that function) to know if ufo.js was already included or not - is that really the case? If so, then maybe ufo.js should be included always (or always if mediaplugin is enabled).

          Show
          Tomasz Muras added a comment - The problem with the filter not working for more than one media file is caused by create_ufo_inline - that embeds ufo.js each time it's called. If ufo.js is included on the page only once, our problem is solved. Now, the note in create_ufo_inline suggests that it's not possible (from that function) to know if ufo.js was already included or not - is that really the case? If so, then maybe ufo.js should be included always (or always if mediaplugin is enabled).
          Hide
          Rossiani Wijaya added a comment -

          Hi Tomasz,

          Just had conversation with Martin and wanted to let you know that we will replace UFO with flowplayer.

          I'm working on this issue right now.

          Show
          Rossiani Wijaya added a comment - Hi Tomasz, Just had conversation with Martin and wanted to let you know that we will replace UFO with flowplayer. I'm working on this issue right now.
          Hide
          Tomasz Muras added a comment -

          Please also have a look (and commit) the attached patch.
          This fixes regular expressions that seem to match too much of the text. The change I've done there is to change this bit:
          .
          into
          [^>]

          I'm not sure what do you mean by replacing UFO with flowplayer, as UFO is used for embedding only. It could be easily done by not using any external JS (like UFO) and either just embed the player or embed JS code that writes correct embedding HTML (this would be done to avoid breaking XHTML compliance but adds dependency on JS enabled.

          I will wait for you to finish your work and then maybe help resolving related issues.

          cheers,
          Tomek

          Show
          Tomasz Muras added a comment - Please also have a look (and commit) the attached patch. This fixes regular expressions that seem to match too much of the text. The change I've done there is to change this bit: . into [^>] I'm not sure what do you mean by replacing UFO with flowplayer, as UFO is used for embedding only. It could be easily done by not using any external JS (like UFO) and either just embed the player or embed JS code that writes correct embedding HTML (this would be done to avoid breaking XHTML compliance but adds dependency on JS enabled. I will wait for you to finish your work and then maybe help resolving related issues. cheers, Tomek
          Hide
          Rossiani Wijaya added a comment -

          Hi Tomasz,

          Please take a look the attached patch and manually add the following .zip files into:

          • lib/mp3player/ - libmp3player.zip
          • filter/mediaplugin/ - filtermdiaplugin.zip

          Feel free to leave any comments.

          Rosie.

          Show
          Rossiani Wijaya added a comment - Hi Tomasz, Please take a look the attached patch and manually add the following .zip files into: lib/mp3player/ - libmp3player.zip filter/mediaplugin/ - filtermdiaplugin.zip Feel free to leave any comments. Rosie.
          Hide
          Tomasz Muras added a comment -

          Hi Rossie,

          That looks just right (I've read the patch but didn't apply it).
          Do you think we could get rid of lib/mp3player altogether? It seems a bit redundant now, that we use flowplayer.
          I think you should commit that all to the repository and I'll do some testing on the latest revision.

          cheers,
          Tomek

          Show
          Tomasz Muras added a comment - Hi Rossie, That looks just right (I've read the patch but didn't apply it). Do you think we could get rid of lib/mp3player altogether? It seems a bit redundant now, that we use flowplayer. I think you should commit that all to the repository and I'll do some testing on the latest revision. cheers, Tomek
          Hide
          Martin Dougiamas added a comment -

          Yep, after looking today I think it's OK for checkin after fixing those few small things I mentioned (eg add OBJECT for filter version so it works non-JS too).

          Show
          Martin Dougiamas added a comment - Yep, after looking today I think it's OK for checkin after fixing those few small things I mentioned (eg add OBJECT for filter version so it works non-JS too).
          Hide
          Rossiani Wijaya added a comment -

          Commit patch to 2.0.

          Show
          Rossiani Wijaya added a comment - Commit patch to 2.0.
          Hide
          Rossiani Wijaya added a comment -

          resolved

          Show
          Rossiani Wijaya added a comment - resolved
          Hide
          Helen Foster added a comment -

          The new flowplayer looks great! Thanks a lot Tomasz and Rosie

          Show
          Helen Foster added a comment - The new flowplayer looks great! Thanks a lot Tomasz and Rosie
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Linking two new issues (MDL-25348 and MDL-25488) related with this. Really I don't know which is the best path to follow here, just listing some possibilities:

          • Allow to define one "default" player size.
          • Try to introspect the file to get it properly sized.
          • Some sort of "auto adjust" option in the player.
          • Add the possibility to define the width and height in the flv URL.
          • Nothing
          • ...

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Linking two new issues ( MDL-25348 and MDL-25488 ) related with this. Really I don't know which is the best path to follow here, just listing some possibilities: Allow to define one "default" player size. Try to introspect the file to get it properly sized. Some sort of "auto adjust" option in the player. Add the possibility to define the width and height in the flv URL. Nothing ... Ciao
          Hide
          Rossiani Wijaya added a comment -

          Re-open issue to fix the default value for flowplayer.

          Show
          Rossiani Wijaya added a comment - Re-open issue to fix the default value for flowplayer.
          Hide
          Rossiani Wijaya added a comment -

          Created patch to set the width and height for the flowplayer.

          The following condition apply to determine player's dimension:
          if flv file dimension is less than 400x320, it will use the file's dimension. Otherwise, it will be set to a default value of 400x320.

          Petr,
          When you have a chance, could you please take a look the patch?

          Thanks
          Rosie

          Show
          Rossiani Wijaya added a comment - Created patch to set the width and height for the flowplayer. The following condition apply to determine player's dimension: if flv file dimension is less than 400x320, it will use the file's dimension. Otherwise, it will be set to a default value of 400x320. Petr, When you have a chance, could you please take a look the patch? Thanks Rosie
          Hide
          Petr Škoda added a comment -

          thanks! going to review it later today

          Show
          Petr Škoda added a comment - thanks! going to review it later today
          Hide
          Petr Škoda added a comment -

          1/ I suppose it would be better to close the file handle after use - fclose()
          2/ all functions in flvlib.php should probably start with prefix "flv_"
          3/ fetching remote files using shell curl does not do proper shell escaping, this could be a security issue
          4/ we can not rely on commandline curl, there would have to be setting for it, there would have to be proxy settings, win compatibility, etc.
          5/ "Add the possibility to define the width and height in the flv URL" it seems to be the most teacher friendly option, but it is not implemented, right? I expected something like http://example.com/xxx.flv#300x200 being converted to flash player 300x200 pixel

          Thanks for the patch, it looks promising, but I am afraid it is not yet ready for commit.

          Petr

          Show
          Petr Škoda added a comment - 1/ I suppose it would be better to close the file handle after use - fclose() 2/ all functions in flvlib.php should probably start with prefix "flv_" 3/ fetching remote files using shell curl does not do proper shell escaping, this could be a security issue 4/ we can not rely on commandline curl, there would have to be setting for it, there would have to be proxy settings, win compatibility, etc. 5/ "Add the possibility to define the width and height in the flv URL" it seems to be the most teacher friendly option, but it is not implemented, right? I expected something like http://example.com/xxx.flv#300x200 being converted to flash player 300x200 pixel Thanks for the patch, it looks promising, but I am afraid it is not yet ready for commit. Petr
          Hide
          Rossiani Wijaya added a comment - - edited

          Petr,

          Currently filter plugin can expect something like http://example.com/xxx.flv?d=300x200. I also change shell curl to php curl_init. (Note: I didn't use moodle curl class because it returned the entire content of the file, which is not needed for this issue).

          One thing I'm not sure is close file handle. Which file are you referring to?

          If you have some spare time, could you take a look the patch? It is available at: https://github.com/rwijaya/moodle/commit/da8028f796b032fe1f54913fd7de73d5aca2e24b

          Show
          Rossiani Wijaya added a comment - - edited Petr, Currently filter plugin can expect something like http://example.com/xxx.flv?d=300x200 . I also change shell curl to php curl_init. (Note: I didn't use moodle curl class because it returned the entire content of the file, which is not needed for this issue). One thing I'm not sure is close file handle. Which file are you referring to? If you have some spare time, could you take a look the patch? It is available at: https://github.com/rwijaya/moodle/commit/da8028f796b032fe1f54913fd7de73d5aca2e24b
          Hide
          Rossiani Wijaya added a comment -

          Add Martin as peer review.

          Show
          Rossiani Wijaya added a comment - Add Martin as peer review.
          Hide
          Rossiani Wijaya added a comment -

          *Adding David to watcher list.

          Hi David,

          When you have a chance, could you review my patch for this issue?

          Thanks
          Rosie

          Show
          Rossiani Wijaya added a comment - *Adding David to watcher list. Hi David, When you have a chance, could you review my patch for this issue? Thanks Rosie
          Hide
          David Mudrak added a comment -

          Hi Rosie,

          I have reviewed https://github.com/rwijaya/moodle/commits/MDL-23870_m20 as it seems to contain some re-work of the commit mentioned above. Please do not forget to rebase the whole branch and meld that revert and reverted commit.

          Comments:

          • the new library flvlib.php must be properly documented with the standard file header and phpdocs for every function
          • I believe that the file handler Petr is talking above is the one you work with in flv_internal_info(). You should fclose that filehandler before you leave the function

          I have not actually tested that it works but once the comments above are reflected, I say +1 for it. Well done Rosie!

          Show
          David Mudrak added a comment - Hi Rosie, I have reviewed https://github.com/rwijaya/moodle/commits/MDL-23870_m20 as it seems to contain some re-work of the commit mentioned above. Please do not forget to rebase the whole branch and meld that revert and reverted commit. Comments: the new library flvlib.php must be properly documented with the standard file header and phpdocs for every function I believe that the file handler Petr is talking above is the one you work with in flv_internal_info(). You should fclose that filehandler before you leave the function I have not actually tested that it works but once the comments above are reflected, I say +1 for it. Well done Rosie!
          Hide
          Jason Ilicic added a comment -

          Cherry-picking the relative commits from the branch is a little tricky which needs tidying up.

          I noticed that you cannot specify the width and height manually in the URL, but this should be a feature. Width and height of the video should be set in the order of: First) check if custom set; Second) check if it exists in meta data; Third) resort to default size setting. It still did not seem to be grabbing the width and height from the FLV meta data either - after opening the FLV file in an editor, it looks like these attributes are endoded as an object of some sort. Further down in the XML, there were other attributes which specified the video frame size which might be able to do so?

          I.e:
          <xmpDM:videoFrameSize
          stDim:w="450"
          stDim:h="240"
          stDim:unit="pixel"/>

          Show
          Jason Ilicic added a comment - Cherry-picking the relative commits from the branch is a little tricky which needs tidying up. I noticed that you cannot specify the width and height manually in the URL, but this should be a feature. Width and height of the video should be set in the order of: First) check if custom set; Second) check if it exists in meta data; Third) resort to default size setting. It still did not seem to be grabbing the width and height from the FLV meta data either - after opening the FLV file in an editor, it looks like these attributes are endoded as an object of some sort. Further down in the XML, there were other attributes which specified the video frame size which might be able to do so? I.e: <xmpDM:videoFrameSize stDim:w="450" stDim:h="240" stDim:unit="pixel"/>
          Hide
          Jason Ilicic added a comment -

          As this patch is not yet resolved and still cannot be applied cleanly, I have made a small patch which will allow video resizing from the URL parameter being specified. This simply limits the size of the container where the embedded object is within.

          Show
          Jason Ilicic added a comment - As this patch is not yet resolved and still cannot be applied cleanly, I have made a small patch which will allow video resizing from the URL parameter being specified. This simply limits the size of the container where the embedded object is within.
          Hide
          Rossiani Wijaya added a comment -

          Hi Petr,

          When you have a chance could you take a look the new patch at https://github.com/rwijaya/moodle/compare/master...MDL-23870_m20

          In this patch, I eliminate the use of CURL for internal url.

          If internal url is used, the system will try to locate and read the file dimension.

          Thanks
          Rosie

          Show
          Rossiani Wijaya added a comment - Hi Petr, When you have a chance could you take a look the new patch at https://github.com/rwijaya/moodle/compare/master...MDL-23870_m20 In this patch, I eliminate the use of CURL for internal url. If internal url is used, the system will try to locate and read the file dimension. Thanks Rosie
          Hide
          Rossiani Wijaya added a comment -

          Discussion with Petr (via jabber):

          (17:09:38) rwijaya@moodle.org: for internal links, it will try to get the file from the server, instead of using curl
          (17:12:54) skodak@moodle.org: do you want to guess the file location from URL?
          (17:13:34) rwijaya@moodle.org: only for interal link
          (17:13:46) skodak@moodle.org: what is "internal"?
          (17:16:52) rwijaya@moodle.org: local domain, if link's domain name is $CFG->wwwroot, then locate the file in the server
          (17:19:50) skodak@moodle.org: we should never read the files directly like this - I am afraid ppl will start to abuse it in new code; bigger problem is that people should not be linking files randomly, and once we fix the format_text() to do the relinking we will have all information there
          (17:23:53) rwijaya@moodle.org: so how should we fix the issue? should we just set the default dimension to 400x320?
          (17:25:09) rwijaya@moodle.org: the dimension can be set manually by adding '?d=###x###' at the end of the link
          (17:25:11) skodak@moodle.org: the first step would be to fix the format_text() to do the @@pluginfile@@ replacement and at the same time it could inspect the files and add the necessary size
          (17:25:24) skodak@moodle.org: or the player itself could resize it automatically
          (17:25:43) skodak@moodle.org: the player knows the size or not?
          (17:26:41) rwijaya@moodle.org: the player doesn't know the size of the file. I set a default value to 400x320,
          (17:27:11) skodak@moodle.org: hmmmm, I think I saw some other player that could detect the size
          (17:27:30) rwijaya@moodle.org: with flowplayer, you have to specify the width and height.
          (17:27:51) skodak@moodle.org: moment please
          (17:34:29) skodak@moodle.org: if it is possible to get the size in PHP then it has to be possible to get the video size in flash - the benefit would be that the flash has the same session, so it would be working 100% compared to using curl without any session or fetching the data from server
          (17:35:28) skodak@moodle.org: unfortunately both of these options (flash resize from inside and format_text) would take a lot longer tom implement,
          (17:36:43) skodak@moodle.org: users must learn to link files only from the current file area (those are stored as @@pluginfile@@ links in text), I do not think we should encourage linking to any file like before
          (17:39:42) rwijaya@moodle.org: PHP can't get the size of flv file. that's why i used curl to read the raw file of flv and determine the file dimension.
          (17:44:06) skodak@moodle.org: PHP is using the curl only to get the binary content, right?
          (17:44:12) skodak@moodle.org: this looks interesting: http://flowplayer.org/forum/2/14168
          (17:44:28) skodak@moodle.org: onStart: function (clip) {
          var w = parseInt(clip.metaData.width, 10),
          h = parseInt(clip.metaData.height, 10);
          $(this.getParent()).css(

          {width: w, height: h}

          );
          }
          (17:44:49) skodak@moodle.org: I am wondering if the clip.metadata.width is the width from the clip
          (17:48:16) rwijaya@moodle.org: i could give it a try
          (17:50:17) skodak@moodle.org: you can just alert the w and h once the script loads
          (18:11:37) rwijaya@moodle.org: it gets the file dimension for last file
          (18:12:08) skodak@moodle.org: is it the dimension you were getting via curl+PHP?
          (18:13:00) rwijaya@moodle.org: yes
          (18:13:48) skodak@moodle.org: so I suppose if you find some way to resize the player in the embedding code it should be possible, right?
          (18:15:46) rwijaya@moodle.org: Yup yup. will try to resizing it again with css

          .....

          issue with flowplayer:
          1. could only retrieve the last player metadata
          2. after retrieving the metadata, it can't be assigned the metadata to flowplayer. i also tried it with document.getElementByid().....width = xxx it won't work either
          flowplayer player dimension is set through the wrapper (div tag).
          we probably could do something like this http://flowplayer.org/demos/plugins/flash/screen-resize.htm
          (16:52:37) skodak@moodle.org: would it be possible to manipulate the CSS instead?
          (16:53:12) skodak@moodle.org: is it possible to set the size of flowplayer through some CSS?
          (16:55:05) skodak@moodle.org: what does it mean "only the last metadata"? what happens if there are multiple videos on one page?
          (16:57:03) rwijaya@moodle.org: I tried with css too, but it doesn't do anything.
          (16:57:33) skodak@moodle.org: can it access the global JS scope?
          (16:57:52) rwijaya@moodle.org: so if you have 10 players on the page. the first 9 players won't returning anything. only the tenth will return the metadata
          (16:58:08) skodak@moodle.org: hmmm
          (16:59:06) skodak@moodle.org: but if we load the plaers one by one or after user click it should get the info, no?
          (17:00:02) rwijaya@moodle.org: the player dimension is set when flowplayer is call
          (17:02:10) rwijaya@moodle.org: the players are available on the page, but since we don't set the dimension for the div/span tag, its hidden from the page
          (17:05:05) skodak@moodle.org: I am confused, the 1. should be solvable by loading first player, getting with x height, fixing width height - the doing the same for the next flv, no?
          (17:05:27) skodak@moodle.org: then grrrrr
          (17:10:34) rwijaya@moodle.org: the player is loading, but file information is not available until the file is play.
          (17:14:23) skodak@moodle.org: where did you implement the resizing code? in cusom flowplayer plugin?
          (17:15:22) rwijaya@moodle.org: i tried in in clip and playlist.
          (17:18:45) rwijaya@moodle.org: when i tried it in custom/addplugins, it doesn't returning anything (tested with alert)
          (17:19:01) skodak@moodle.org: what event did you use?
          (17:19:31) rwijaya@moodle.org: OnStart, OnBegin, OnLoad
          (17:19:57) rwijaya@moodle.org: onReady (just to try it)
          (17:20:03) skodak@moodle.org: I am looking for the complete list of events
          (17:20:21) rwijaya@moodle.org: http://flowplayer.org/documentation/events/clip.html
          (17:20:50) skodak@moodle.org: onMetaData ?
          (17:21:05) rwijaya@moodle.org: yup tried that too. and onUpdate
          (17:22:11) skodak@moodle.org: the onMetadata() - you did not get that event or no information was there?
          (17:22:49) rwijaya@moodle.org: it didn't get to that event
          (17:23:16) skodak@moodle.org: when there was only one player in page?
          (17:23:49) rwijaya@moodle.org: only onStart returning the alert, but just once.
          (17:24:53) rwijaya@moodle.org: not returning anything onMetadata
          (17:25:12) skodak@moodle.org: why would they add these things if they did not work?
          (17:25:30) skodak@moodle.org: did you try different browser/OS?
          (17:25:46) rwijaya@moodle.org: i tried firefox and chrome. but not IE
          (17:26:02) skodak@moodle.org: in Windows?
          (17:26:13) rwijaya@moodle.org: ubuntu
          (17:26:44) skodak@moodle.org: hmmm, could you please try it in one of the virtual machines with Windows?
          (17:27:00) skodak@moodle.org: ask jordan how to get one of those VMs if necessary
          (17:27:55) rwijaya@moodle.org: ok. i'm going to tested it.
          (17:28:07) skodak@moodle.org: thanks a lot
          (17:56:30) rwijaya@moodle.org: this is the result with IE 8 and 9. (tested with 4 players on the page):
          onStart:
          IE9: alert with value (occur 1 time)
          IE8: none

          onLoad:
          IE9: none
          IE8: none

          onMetadata
          IE9: none
          IE8: none

          onUpdate
          IE9: none
          IE8: none

          onBegin
          IE9: alert with no value (occur 1 time)
          IE8: none

          Show
          Rossiani Wijaya added a comment - Discussion with Petr (via jabber): (17:09:38) rwijaya@moodle.org: for internal links, it will try to get the file from the server, instead of using curl (17:12:54) skodak@moodle.org: do you want to guess the file location from URL? (17:13:34) rwijaya@moodle.org: only for interal link (17:13:46) skodak@moodle.org: what is "internal"? (17:16:52) rwijaya@moodle.org: local domain, if link's domain name is $CFG->wwwroot, then locate the file in the server (17:19:50) skodak@moodle.org: we should never read the files directly like this - I am afraid ppl will start to abuse it in new code; bigger problem is that people should not be linking files randomly, and once we fix the format_text() to do the relinking we will have all information there (17:23:53) rwijaya@moodle.org: so how should we fix the issue? should we just set the default dimension to 400x320? (17:25:09) rwijaya@moodle.org: the dimension can be set manually by adding '?d=###x###' at the end of the link (17:25:11) skodak@moodle.org: the first step would be to fix the format_text() to do the @@pluginfile@@ replacement and at the same time it could inspect the files and add the necessary size (17:25:24) skodak@moodle.org: or the player itself could resize it automatically (17:25:43) skodak@moodle.org: the player knows the size or not? (17:26:41) rwijaya@moodle.org: the player doesn't know the size of the file. I set a default value to 400x320, (17:27:11) skodak@moodle.org: hmmmm, I think I saw some other player that could detect the size (17:27:30) rwijaya@moodle.org: with flowplayer, you have to specify the width and height. (17:27:51) skodak@moodle.org: moment please (17:34:29) skodak@moodle.org: if it is possible to get the size in PHP then it has to be possible to get the video size in flash - the benefit would be that the flash has the same session, so it would be working 100% compared to using curl without any session or fetching the data from server (17:35:28) skodak@moodle.org: unfortunately both of these options (flash resize from inside and format_text) would take a lot longer tom implement, (17:36:43) skodak@moodle.org: users must learn to link files only from the current file area (those are stored as @@pluginfile@@ links in text), I do not think we should encourage linking to any file like before (17:39:42) rwijaya@moodle.org: PHP can't get the size of flv file. that's why i used curl to read the raw file of flv and determine the file dimension. (17:44:06) skodak@moodle.org: PHP is using the curl only to get the binary content, right? (17:44:12) skodak@moodle.org: this looks interesting: http://flowplayer.org/forum/2/14168 (17:44:28) skodak@moodle.org: onStart: function (clip) { var w = parseInt(clip.metaData.width, 10), h = parseInt(clip.metaData.height, 10); $(this.getParent()).css( {width: w, height: h} ); } (17:44:49) skodak@moodle.org: I am wondering if the clip.metadata.width is the width from the clip (17:48:16) rwijaya@moodle.org: i could give it a try (17:50:17) skodak@moodle.org: you can just alert the w and h once the script loads (18:11:37) rwijaya@moodle.org: it gets the file dimension for last file (18:12:08) skodak@moodle.org: is it the dimension you were getting via curl+PHP? (18:13:00) rwijaya@moodle.org: yes (18:13:48) skodak@moodle.org: so I suppose if you find some way to resize the player in the embedding code it should be possible, right? (18:15:46) rwijaya@moodle.org: Yup yup. will try to resizing it again with css ..... issue with flowplayer: 1. could only retrieve the last player metadata 2. after retrieving the metadata, it can't be assigned the metadata to flowplayer. i also tried it with document.getElementByid().....width = xxx it won't work either flowplayer player dimension is set through the wrapper (div tag). we probably could do something like this http://flowplayer.org/demos/plugins/flash/screen-resize.htm (16:52:37) skodak@moodle.org: would it be possible to manipulate the CSS instead? (16:53:12) skodak@moodle.org: is it possible to set the size of flowplayer through some CSS? (16:55:05) skodak@moodle.org: what does it mean "only the last metadata"? what happens if there are multiple videos on one page? (16:57:03) rwijaya@moodle.org: I tried with css too, but it doesn't do anything. (16:57:33) skodak@moodle.org: can it access the global JS scope? (16:57:52) rwijaya@moodle.org: so if you have 10 players on the page. the first 9 players won't returning anything. only the tenth will return the metadata (16:58:08) skodak@moodle.org: hmmm (16:59:06) skodak@moodle.org: but if we load the plaers one by one or after user click it should get the info, no? (17:00:02) rwijaya@moodle.org: the player dimension is set when flowplayer is call (17:02:10) rwijaya@moodle.org: the players are available on the page, but since we don't set the dimension for the div/span tag, its hidden from the page (17:05:05) skodak@moodle.org: I am confused, the 1. should be solvable by loading first player, getting with x height, fixing width height - the doing the same for the next flv, no? (17:05:27) skodak@moodle.org: then grrrrr (17:10:34) rwijaya@moodle.org: the player is loading, but file information is not available until the file is play. (17:14:23) skodak@moodle.org: where did you implement the resizing code? in cusom flowplayer plugin? (17:15:22) rwijaya@moodle.org: i tried in in clip and playlist. (17:18:45) rwijaya@moodle.org: when i tried it in custom/addplugins, it doesn't returning anything (tested with alert) (17:19:01) skodak@moodle.org: what event did you use? (17:19:31) rwijaya@moodle.org: OnStart, OnBegin, OnLoad (17:19:57) rwijaya@moodle.org: onReady (just to try it) (17:20:03) skodak@moodle.org: I am looking for the complete list of events (17:20:21) rwijaya@moodle.org: http://flowplayer.org/documentation/events/clip.html (17:20:50) skodak@moodle.org: onMetaData ? (17:21:05) rwijaya@moodle.org: yup tried that too. and onUpdate (17:22:11) skodak@moodle.org: the onMetadata() - you did not get that event or no information was there? (17:22:49) rwijaya@moodle.org: it didn't get to that event (17:23:16) skodak@moodle.org: when there was only one player in page? (17:23:49) rwijaya@moodle.org: only onStart returning the alert, but just once. (17:24:53) rwijaya@moodle.org: not returning anything onMetadata (17:25:12) skodak@moodle.org: why would they add these things if they did not work? (17:25:30) skodak@moodle.org: did you try different browser/OS? (17:25:46) rwijaya@moodle.org: i tried firefox and chrome. but not IE (17:26:02) skodak@moodle.org: in Windows? (17:26:13) rwijaya@moodle.org: ubuntu (17:26:44) skodak@moodle.org: hmmm, could you please try it in one of the virtual machines with Windows? (17:27:00) skodak@moodle.org: ask jordan how to get one of those VMs if necessary (17:27:55) rwijaya@moodle.org: ok. i'm going to tested it. (17:28:07) skodak@moodle.org: thanks a lot (17:56:30) rwijaya@moodle.org: this is the result with IE 8 and 9. (tested with 4 players on the page): onStart: IE9: alert with value (occur 1 time) IE8: none onLoad: IE9: none IE8: none onMetadata IE9: none IE8: none onUpdate IE9: none IE8: none onBegin IE9: alert with no value (occur 1 time) IE8: none
          Hide
          Derek Chirnside added a comment -

          How is progress on this? What does a PULL mean?

          Show
          Derek Chirnside added a comment - How is progress on this? What does a PULL mean?
          Hide
          Rossiani Wijaya added a comment -

          UPDATE

          Petr is currently working on updating the mediaplugin filter page (MDL-26456), which include reducing the default value for the flowplayer. Two pull requests are expected to be submitted in the following week. The first pull request will address new regex for 1.9 and second pull will cover major changes in 2.0.x.

          After the above patches has been submitted, I will then continue to work on fetching the file dimension.

          Derek,
          PULL means patch has been created and submitted for review (by senior developer). If it passed the review stage, it will be QA by tester and submitted to integration. if it fails the review or QA stage, it will go back to its original status.

          Rosie

          Show
          Rossiani Wijaya added a comment - UPDATE Petr is currently working on updating the mediaplugin filter page ( MDL-26456 ), which include reducing the default value for the flowplayer. Two pull requests are expected to be submitted in the following week. The first pull request will address new regex for 1.9 and second pull will cover major changes in 2.0.x. After the above patches has been submitted, I will then continue to work on fetching the file dimension. Derek, PULL means patch has been created and submitted for review (by senior developer). If it passed the review stage, it will be QA by tester and submitted to integration. if it fails the review or QA stage, it will go back to its original status. Rosie
          Hide
          Derek Chirnside added a comment -

          Cool.
          What sort of timeline for this?
          Do you need help with QA?
          -Derek

          Show
          Derek Chirnside added a comment - Cool. What sort of timeline for this? Do you need help with QA? -Derek
          Hide
          Petr Škoda added a comment -

          Hello,
          I am going to submit new PULL request with reworked flow player integration this week, it will probably land in main git repo in one of the next weeks.

          Thanks everybody.

          Show
          Petr Škoda added a comment - Hello, I am going to submit new PULL request with reworked flow player integration this week, it will probably land in main git repo in one of the next weeks. Thanks everybody.
          Hide
          Derek Chirnside added a comment -

          Petr, I've just had a flurry of notifications!! You must be working hard.
          Just in case you missed this: http://moodle.org/mod/forum/discuss.php?d=169935
          I quote:

          Hi,

          I am using Moodle 2.0 (Build: 20101214) for streaming lecture videos. The vids are encoded as FLV (flash video), H.263 video and MP3 audio. Aspect ratio is 4:3 (640x480). I upload the vids to a repository, and use the "add a file" feature.

          The built-in player (flowplayer 3.2.3) works nicely, until you attempt fullscreen viewing. Flowplayer apparently has the scaling set to 'scale', which fills the screen and ignores aspect ratios.

          from http://flowplayer.org/documentation/configuration/clips.html, I see that available scaling options are:

          • fit: Fit to window by preserving the aspect ratio encoded in the file's metadata.
          • half: Half-size (preserves aspect ratio)
          • orig: Use the dimensions encoded in the file. If the video is too big for the available space, the video is scaled using the 'fit' option.
          • scale: Scale the video to fill all available space. Ignores the dimensions in the metadata. This is the default setting.

          My question is, how do I change this setting? where would I find it?

          Thanks in advance for your help!!

          • Jesse D
          Show
          Derek Chirnside added a comment - Petr, I've just had a flurry of notifications!! You must be working hard. Just in case you missed this: http://moodle.org/mod/forum/discuss.php?d=169935 I quote: – Hi, I am using Moodle 2.0 (Build: 20101214) for streaming lecture videos. The vids are encoded as FLV (flash video), H.263 video and MP3 audio. Aspect ratio is 4:3 (640x480). I upload the vids to a repository, and use the "add a file" feature. The built-in player (flowplayer 3.2.3) works nicely, until you attempt fullscreen viewing. Flowplayer apparently has the scaling set to 'scale', which fills the screen and ignores aspect ratios. from http://flowplayer.org/documentation/configuration/clips.html , I see that available scaling options are: fit: Fit to window by preserving the aspect ratio encoded in the file's metadata. half: Half-size (preserves aspect ratio) orig: Use the dimensions encoded in the file. If the video is too big for the available space, the video is scaled using the 'fit' option. scale: Scale the video to fill all available space. Ignores the dimensions in the metadata. This is the default setting. My question is, how do I change this setting? where would I find it? Thanks in advance for your help!! Jesse D
          Hide
          Petr Škoda added a comment -

          Thanks for the scaling info, I have added it to my patch.

          Show
          Petr Škoda added a comment - Thanks for the scaling info, I have added it to my patch.
          Hide
          Petr Škoda added a comment -

          Hello,
          new flow player integration is available in the latest weekly build, the old players and embedding scripts were removed completely. The flash videos are being resized automatically using the built-in metadata.

          Thanks for the report, patches and participation here.

          Please report any remaining issues as new bugs, ciao!

          Petr

          Show
          Petr Škoda added a comment - Hello, new flow player integration is available in the latest weekly build, the old players and embedding scripts were removed completely. The flash videos are being resized automatically using the built-in metadata. Thanks for the report, patches and participation here. Please report any remaining issues as new bugs, ciao! Petr

            People

            • Votes:
              7 Vote for this issue
              Watchers:
              16 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: