Comment on attachment 611716 New patch addressing 1st review comments + misc fixes. See comment #263.
Review of attachment 611716: ----------------------------------------------------------------- Looks good, just minor comments. r+ with those, and the configure change you mentioned to address the gstreamer version. I don't think it's required that all mochitests pass to land your patch since it's disabled by default - it isn't built without --enable-gstreamer. Followup bugs can be raised to address the failing tests and additional functionality. Additional tests would be good for H.264 encoded files and anything else specific to the backend that you think would be useful. Have you tried the reftests that compare the result of the decoded video frames with a reference image? ::: content/media/gstreamer/nsGStreamerReader.cpp @@ +185,2 @@ > > + /* We do 3 attemtps here: decoding audio and video, decoding video only, Minor: Spelling of 'attempts'. @@ +322,5 @@ > + mDecoder->NotifyBytesConsumed(mByteOffset - mLastReportedByteOffset); > + mLastReportedByteOffset = mByteOffset; > +} > + > +bool nsGStreamerReader::WaitForDecodedData(int *counter) Minor: Change 'counter' to 'aCounter'. @@ +613,5 @@ > > void nsGStreamerReader::NeedData(GstAppSrc *aSrc, guint aLength) > { > + if (aLength == -1) > + aLength = 50 * 1024; Can you make this magic number (50*1024) a define/constant somewhere. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/412647 Title: Firefox is not able to play mp4 <video> tags To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/412647/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs