Closed Bug 1034308 Opened 6 years ago Closed 5 years ago

RtspMediaResource uses the NS_DECL_NSISTREAMINGPROTOCOLLISTENER macro, but does not inherit from nsIStreamingProtocolListener

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: khuey, Assigned: khuey)

References

Details

Attachments

(1 file)

This fails to compile once we add MOZ_OVERRIDE annotations to the macros produced by the XPIDL compiler.  We should either write out the functions by hand or explicitly inherit from nsIStreamingProtocolListener.
Attached patch PatchSplinter Review
Assignee: nobody → khuey
Status: NEW → ASSIGNED
Attachment #8467348 - Flags: review?(ettseng)
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #0)
> This fails to compile once we add MOZ_OVERRIDE annotations to the macros
> produced by the XPIDL compiler.  We should either write out the functions by
> hand or explicitly inherit from nsIStreamingProtocolListener.

Kyle, thanks for you patch!

Conceptually, we want to keep the relationships between RtspMediaResource and nsIStreamingProtocolListener being loosely coupled. We wouldn't like to make RtspMediaResource inherit from nsIStreamingProtocolListener. So, declare those functions by hand would be better. :)
Comment on attachment 8467348 [details] [diff] [review]
Patch

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

Looks good! Thanks, Kyle.

::: content/media/RtspMediaResource.h
@@ +230,1 @@
>  

nit: Please remove the trailing space in this line. :)
Attachment #8467348 - Flags: review?(ettseng) → review+
Comment on attachment 8467348 [details] [diff] [review]
Patch

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

::: content/media/RtspMediaResource.h
@@ +225,5 @@
> +  nsresult OnMediaDataAvailable(uint8_t aIndex, const nsACString& aData,
> +                                uint32_t aLength, uint32_t aOffset,
> +                                nsIStreamingProtocolMetaData* aMeta);
> +  nsresult OnConnected(uint8_t aIndex, nsIStreamingProtocolMetaData* aMeta);
> +  nsresult OnDisconnected(uint8_t aIndex, nsresult aReason); 

Well, I mean this line.
(In reply to Ethan Tseng [:ethan] from comment #4)
> Comment on attachment 8467348 [details] [diff] [review]
> Patch
> 
> Review of attachment 8467348 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/media/RtspMediaResource.h
> @@ +225,5 @@
> > +  nsresult OnMediaDataAvailable(uint8_t aIndex, const nsACString& aData,
> > +                                uint32_t aLength, uint32_t aOffset,
> > +                                nsIStreamingProtocolMetaData* aMeta);
> > +  nsresult OnConnected(uint8_t aIndex, nsIStreamingProtocolMetaData* aMeta);
> > +  nsresult OnDisconnected(uint8_t aIndex, nsresult aReason); 
> 
> Well, I mean this line.

Fixed.

https://hg.mozilla.org/integration/b2g-inbound/rev/1c93a1e58c5c
https://hg.mozilla.org/mozilla-central/rev/1c93a1e58c5c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.