1418868 - Remove IndiceWrapper abstract class

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P3)

Description

[Alfredo Yang (:alfredo)]

Since stagefright is removed, we don't need an abstract layer anymore.

Comment 1

[Alfredo Yang (:alfredo)]

Attachment: Bug 1418868 - remove IndiceWrapper abstract layer.

Review commit: https://reviewboard.mozilla.org/r/201116/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201116/

Comment 2

[Matthew Gregan [:kinetik]]

Comment on attachment 8929957 [details]
Bug 1418868 - remove IndiceWrapper abstract layer.

https://reviewboard.mozilla.org/r/201116/#review206244

::: dom/media/mp4/MP4Metadata.h:20
(Diff revision 1)
>  #include "ByteStream.h"
>  #include "mp4parse.h"
>  
>  namespace mozilla {
>  
> -class IndiceWrapper {
> +// the owner of mIndice is rust mp4 paser, so lifetime of this class

the -> The

paser -> parser

::: dom/media/mp4/MP4Metadata.h:21
(Diff revision 1)
>  #include "mp4parse.h"
>  
>  namespace mozilla {
>  
> -class IndiceWrapper {
> +// the owner of mIndice is rust mp4 paser, so lifetime of this class
> +// SHOULD NOT longer than rust parser.

This comment doesn't seem right.  mUnique is a UniquePtr, which is owning, and it's allocated inside IndiceWrapper's ctor and the members of aRustIndice are copied over.  So there's no connection between an IndiceWrapper and mp4 parser's lifetime, right?

It seems like now we could make mIndice be a direct member rather than allocating it, too.

Comment 3

[Alfredo Yang (:alfredo)]

(In reply to Matthew Gregan [:kinetik] from comment #2)
> > -class IndiceWrapper {
> > +// the owner of mIndice is rust mp4 paser, so lifetime of this class
> > +// SHOULD NOT longer than rust parser.
> 
> This comment doesn't seem right.  mUnique is a UniquePtr, which is owning,
> and it's allocated inside IndiceWrapper's ctor and the members of
> aRustIndice are copied over.  So there's no connection between an
> IndiceWrapper and mp4 parser's lifetime, right?

Yes, the memory owned by mp4 parser is the raw data from rust Vec. I'll update it.

> 
> It seems like now we could make mIndice be a direct member rather than
> allocating it, too.

Will do.

Comment 4

[Alfredo Yang (:alfredo)]

Comment on attachment 8929957 [details]
Bug 1418868 - remove IndiceWrapper abstract layer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201116/diff/1-2/

Comment 5

[Matthew Gregan [:kinetik]]

Comment on attachment 8929957 [details]
Bug 1418868 - remove IndiceWrapper abstract layer.

https://reviewboard.mozilla.org/r/201116/#review206824

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05e3e4ff97a5
remove IndiceWrapper abstract layer. r=kinetik

Comment 7

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/05e3e4ff97a5