WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
169567
Make classes used by Media Stream encode/decode friendly
https://bugs.webkit.org/show_bug.cgi?id=169567
Summary
Make classes used by Media Stream encode/decode friendly
Jer Noble
Reported
2017-03-13 14:02:00 PDT
Make classes used by Media Stream encode/decode friendly
Attachments
Patch
(15.72 KB, patch)
2017-03-13 14:04 PDT
,
Jer Noble
eric.carlson
: review+
Details
Formatted Diff
Diff
Patch for landing
(16.60 KB, patch)
2017-03-13 23:54 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing
(16.32 KB, patch)
2017-03-14 00:01 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Jer Noble
Comment 1
2017-03-13 14:04:03 PDT
Created
attachment 304297
[details]
Patch
Eric Carlson
Comment 2
2017-03-13 15:43:56 PDT
Comment on
attachment 304297
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=304297&action=review
> Source/WebCore/Modules/mediastream/MediaConstraintsImpl.h:70 > +template<class Encoder> > +void MediaConstraintsData::encode(Encoder& encoder) const > +{ > + encoder << mandatoryConstraints << advancedConstraints << isValid; > +} > + > +template<class Decoder> > +bool MediaConstraintsData::decode(Decoder& decoder, MediaConstraintsData& data) > +{ > + return decoder.decode(data.mandatoryConstraints) > + && decoder.decode(data.advancedConstraints) > + && decoder.decode(data.isValid); > +}
These are nice, but we already have an encoder and decoder in WebCoreArgumentCoders. Why don't you either delete those or replace them with these?
> Source/WebCore/platform/mediastream/CaptureDevice.h:92 > +void CaptureDevice::encode(Encoder& encoder) const > +{ > + encoder << m_persistentId > + << m_label > + << m_groupId > + << m_enabled; > + encoder.encodeEnum(m_type); > +} > + > +template<class Decoder> > +bool CaptureDevice::decode(Decoder& decoder, CaptureDevice& device) > +{ > + return decoder.decode(device.m_persistentId) > + && decoder.decode(device.m_label) > + && decoder.decode(device.m_groupId) > + && decoder.decode(device.m_enabled) > + && decoder.decodeEnum(device.m_type); > +}
Ditto.
Jer Noble
Comment 3
2017-03-13 23:49:04 PDT
(In reply to
comment #2
)
> Comment on
attachment 304297
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=304297&action=review
> > > Source/WebCore/Modules/mediastream/MediaConstraintsImpl.h:70 > > +template<class Encoder> > > +void MediaConstraintsData::encode(Encoder& encoder) const > > +{ > > + encoder << mandatoryConstraints << advancedConstraints << isValid; > > +} > > + > > +template<class Decoder> > > +bool MediaConstraintsData::decode(Decoder& decoder, MediaConstraintsData& data) > > +{ > > + return decoder.decode(data.mandatoryConstraints) > > + && decoder.decode(data.advancedConstraints) > > + && decoder.decode(data.isValid); > > +} > > These are nice, but we already have an encoder and decoder in > WebCoreArgumentCoders. Why don't you either delete those or replace them > with these? > > > Source/WebCore/platform/mediastream/CaptureDevice.h:92 > > +void CaptureDevice::encode(Encoder& encoder) const > > +{ > > + encoder << m_persistentId > > + << m_label > > + << m_groupId > > + << m_enabled; > > + encoder.encodeEnum(m_type); > > +} > > + > > +template<class Decoder> > > +bool CaptureDevice::decode(Decoder& decoder, CaptureDevice& device) > > +{ > > + return decoder.decode(device.m_persistentId) > > + && decoder.decode(device.m_label) > > + && decoder.decode(device.m_groupId) > > + && decoder.decode(device.m_enabled) > > + && decoder.decodeEnum(device.m_type); > > +} > > Ditto.
Good point!
Jer Noble
Comment 4
2017-03-13 23:54:36 PDT
Created
attachment 304359
[details]
Patch for landing
Jer Noble
Comment 5
2017-03-14 00:01:07 PDT
Created
attachment 304360
[details]
Patch for landing
Jer Noble
Comment 6
2017-03-14 13:50:59 PDT
Committed
r213936
: <
http://trac.webkit.org/changeset/213936
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug