778105 - nsOggReader::GetBuffered treats PageSyncResult as nsresult

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

There's a "return PAGE_SYNC_ERROR;" in there, added by bug 584615.  This is wrong -- PAGE_SYNC_ERROR is equal to 1, which isn't a valid nsresult, and will be interpreted as successful.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Thanks for filing this bug. Thankfully is return value doesn't propagate all the way up to JS.

We hit this case when we detect a page from a stream we've not encountered before (like a new link in a chain or a live stream).

I think we can just return NS_OK in this case.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch

Comment 3

[cajbir (:cajbir)]

Comment on attachment 647736 [details] [diff] [review]
Patch

Review of attachment 647736 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/ogg/nsOggReader.cpp
@@ +1621,5 @@
>          continue;
>        }
>        else {
>          // Page is for a stream we don't know about (possibly a chained
>          // ogg), return an error.

Might want to change the comment since we're no longer returning an error.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

With comment changed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3fc0a0b7fdda

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3fc0a0b7fdda