Closed Bug 1219984 Opened 6 years ago Closed 6 years ago

Add support for multiple arguments to MediaEventSource

Categories

(Core :: Audio/Video: Playback, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(2 files)

Instead of saying:

struct SomeStruct {
  int foo;
  int bar;
} someEvent;

someProducer.Notify(someEvent);

you can say:

int foo = 1;
int bar = 2;
someProducer.Notify(foo, bar);

without defining a new struct.
Assignee: nobody → jwwang
You probably have a good use case for that, but just in case:
For simple structs, you can create them on the spot using aggregate initialization, e.g.:
> someProducer.Notify({foo, bar});
Bug 1219984. Part 1 - remove EventPassMode::Both. In order to support multiple arguments, all arguments must be either moved or copied. r=kinetik.
Attachment #8681090 - Flags: review?(kinetik)
Bug 1219984. Part 2 - add support for multiple arguments. r=kinetik.
Attachment #8681091 - Flags: review?(kinetik)
Comment on attachment 8681090 [details]
MozReview Request: Bug 1219984. Part 1 - remove EventPassMode::Both. In order to support multiple arguments, all arguments must be either moved or copied. r=kinetik.

https://reviewboard.mozilla.org/r/23765/#review21327
Attachment #8681090 - Flags: review?(kinetik) → review+
Comment on attachment 8681091 [details]
MozReview Request: Bug 1219984. Part 2 - add support for multiple arguments. r=kinetik.

https://reviewboard.mozilla.org/r/23767/#review21329
Attachment #8681091 - Flags: review?(kinetik) → review+
Thanks for the review!
(In reply to Gerald Squelart [:gerald] from comment #1)
> You probably have a good use case for that, but just in case:
> For simple structs, you can create them on the spot using aggregate
> initialization, e.g.:
> > someProducer.Notify({foo, bar});

It looks like initializer_list doesn't play well with perfect forwarding.

struct SomeStruct {
    int foo;
    int bar;
};

void DoSomething(const SomeStruct& a) {
    std::cout << "foo=" << a.foo << "\n";
    std::cout << "bar=" << a.bar << "\n";
}

template <typename... Ts>
void ForwardSomething(Ts&&... args) {
    DoSomething(std::forward<Ts>(args)...);
}

int main()
{
    SomeStruct s {1, 2};
    
    DoSomething(s);
    DoSomething({3, 4});
    ForwardSomething(s);
    //ForwardSomething({3, 4}); // error!
}
Btw, I think we should support |someProducer.Notify({foo, bar})|. I will file a new bug to fix it.
Blocks: 1220514
(In reply to JW Wang [:jwwang] from comment #7)
> (In reply to Gerald Squelart [:gerald] from comment #1)
> > You probably have a good use case for that, but just in case:
> > For simple structs, you can create them on the spot using aggregate
> > initialization, e.g.:
> > > someProducer.Notify({foo, bar});
> 
> It looks like initializer_list doesn't play well with perfect forwarding.
> 
> struct SomeStruct {..};
> void DoSomething(const SomeStruct& a) {..}
> template <typename... Ts>
> void ForwardSomething(Ts&&... args) {
>   DoSomething(std::forward<Ts>(args)...);
> }
> int main()
> {   ..
>     //ForwardSomething({3, 4}); // error!
> }

Good point.
That's due to the template automatic type deduction seeing '{..}' as an initializer_list, in absence of any direct clue.
'ForwardSomething<SomeStruct>({3, 4});' or 'ForwardSomething(SomeStruct{3, 4})' work fine.


(In reply to JW Wang [:jwwang] from comment #9)
> Btw, I think we should support |someProducer.Notify({foo, bar})|. I will
> file a new bug to fix it.

I'm not sure supporting initializer_list for a function accepting a struct is necessarily a good idea, as struct's may have different types of members while initializer_list only supports one type for all values; it usually makes most sense for things like initializing a container of values with the same type.
But once again, you probably know more about use cases in this code, where it would make sense.
(In reply to Gerald Squelart [:gerald] from comment #10)
> I'm not sure supporting initializer_list for a function accepting a struct
> is necessarily a good idea, as struct's may have different types of members
> while initializer_list only supports one type for all values;

It will not be very useful to requiring all members to be the same type. However, since |ForwardSomething(SomeStruct{3, 4})| works fine, we don't need to fix bug 1220514 until we figure out a good way to do that.
https://hg.mozilla.org/mozilla-central/rev/2a027a19e086
https://hg.mozilla.org/mozilla-central/rev/ca24bcc14866
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.