Bug 169567

Summary: Make classes used by Media Stream encode/decode friendly
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 169609    
Attachments:
Description Flags
Patch
eric.carlson: review+
Patch for landing
none
Patch for landing none

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+
Patch for landing (16.60 KB, patch)
2017-03-13 23:54 PDT, Jer Noble
no flags
Patch for landing (16.32 KB, patch)
2017-03-14 00:01 PDT, Jer Noble
no flags
Jer Noble
Comment 1 2017-03-13 14:04:03 PDT
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
Note You need to log in before you can comment on or make changes to this bug.