#481 ✓ staged
Scott Downe

creating default end times based on duration can fail

Reported by Scott Downe | April 18th, 2011 @ 11:47 AM | in 0.7

In some cases, like this file:

    <audio id="media" controls autobuffer>

      <!-- Ogg Vorbis -->
      <source src="http://api.soundcloud.com/tracks/13607892/download?client_id=CHAyhB5IisvLqqzGYNYbmA" />
      <!-- MP3 -->
      <source src="http://api.soundcloud.com/tracks/13607892/stream?client_id=CHAyhB5IisvLqqzGYNYbmA" />
    </audio>

duration will never exist, reported as NaN. If you create a track with no out time, it defaults to options.end = this.duration();

We should consider doing a check like we do with the internal track we pad the track arrays with: videoDurationPlus = duration != duration ? Number.MAX_VALUE : duration + 1;

I am going to make a working reduced test of this failing.

The fix should also have a unit test.

Comments and changes to this ticket

  • Scott Downe

    Scott Downe April 18th, 2011 @ 11:57 AM

    simple working reduced test case:

    <html>
    <head>
    <script src="https://github.com/annasob/popcorn-js/raw/0.5/popcorn.js"></script>
    
    <script>
    
        document.addEventListener('DOMContentLoaded', function () {
    
          Popcorn.plugin("durationfail", {
            _setup: function(options) {
              console.log(options.end);
            },
            start: function(event, options) {
    
            },
            end: function(event, options) {
    
            }
          });
    
          var p = Popcorn('#media')
          .play() 
          .durationfail({
            start: 0, // seconds
            apikey: "CHAyhB5IisvLqqzGYNYbmA",
            mediaid: "13607892"
          })
          var p = Popcorn('#video')
          .play() 
          .durationfail({
            start: 0, // seconds
            apikey: "CHAyhB5IisvLqqzGYNYbmA",
            mediaid: "13607892"
          })
        }, false);
    
    </script>
    
    </head>
    <body>
        <audio id="media" controls autobuffer>
    
          <!-- Ogg Vorbis -->
          <source src="http://api.soundcloud.com/tracks/13607892/download?client_id=CHAyhB5IisvLqqzGYNYbmA" />
          <!-- MP3 -->
          <!--<source src="http://api.soundcloud.com/tracks/13607892/stream?client_id=CHAyhB5IisvLqqzGYNYbmA" />-->
        </audio>
    
        <video id="video" src="http://popcornjs.org/tryme.ogv"></video>
    
    </body>
    </html>
    
  • cadecairos

    cadecairos May 31st, 2011 @ 03:12 PM

    • State changed from “new” to “peer-review-requested”
    • Assigned user set to “Scott Downe”

    Got a fix for this, along with tests to enforce the behaviour.

    here is the commit

    The branch is t481

  • Rick

    Rick May 31st, 2011 @ 05:13 PM

    • Milestone set to 0.7
    • State changed from “peer-review-requested” to “review-needs-work”
    • Assigned user changed from “Scott Downe” to “cadecairos”
    • Milestone order changed from “53” to “0”

    Can reduce that too:

    
    options.end = this.duration() || Number.MAX_VALUE;
    

    baseplayer shouldn't be added to the core's test suite

  • cadecairos

    cadecairos May 31st, 2011 @ 05:28 PM

    • State changed from “review-needs-work” to “peer-review-requested”
    • Assigned user changed from “cadecairos” to “Rick”

    new commit

    I've reduced the code and changed the test that used baseplayer to use a source-less audio element instead.

  • Rick

    Rick May 31st, 2011 @ 06:37 PM

    • State changed from “peer-review-requested” to “super-review-requested”

    Nice! PR+

    Tested: [] test suite;

    Passing in:

    • FF 3.6, 4.x, 6.x

    • Chrome 11, 12, 13

  • Rick

    Rick June 2nd, 2011 @ 09:29 AM

    • Assigned user changed from “Rick” to “Scott Downe”
  • Scott Downe

    Scott Downe June 2nd, 2011 @ 09:56 AM

    • State changed from “super-review-requested” to “review-looks-good”
    • Assigned user changed from “Scott Downe” to “Jon Buckley”

    Tested both regular and complete tests. expected errors in complete, regular full pass.

    Linting.

    Demo works.

  • Jon Buckley

    Jon Buckley June 2nd, 2011 @ 06:00 PM

    • State changed from “review-looks-good” to “staged”

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Popcorn.js is an HTML5 video framework that lets you bring elements of the web into your videos.

Popcorn.js is a project of Web Made Movies, Mozilla's Open Video Lab.

Shared Ticket Bins

Referenced by

Pages