[Bug 412647]

[Chris Double]

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 Mozilla
Bugs, which is subscribed to Mozilla.
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