[prev in list] [next in list] [prev in thread] [next in thread] 

List:       helix-datatype-dev
Subject:    [datatype-dev] CN - Implement Plugin interface in AMR
From:       "Alok Jain" <alokj () real ! com>
Date:       2009-03-24 9:45:36
Message-ID: 00a301c9ac63$9e3c9500$1e01a8c0 () AlokSystem
[Download RAW message or body]

Thanks Eric,
Suggested changes have been incorporated and Now checked into HEAD.
--Alok

----- Original Message ----- 
From: "Eric Hyche" <ehyche@real.com>
To: "'Alok Jain'" <alokj@real.com>; <datatype-dev@helixcommunity.org>
Sent: Monday, March 23, 2009 7:01 PM
Subject: RE: CR - Implement Plugin interface in AMR


> Alok,
>
> Here are my comments:
>
> +const char* AMRPacketizer::zm_pDescription    = "RealNetworks AMR 
> Packetizer Plugin";
> +const char* AMRPacketizer::zm_pCopyright      = HXVER_COPYRIGHT;
> +const char* AMRPacketizer::zm_pMoreInfoURL    = HXVER_MOREINFO;
>
> These should be "const char* const" instead of "const char*".
>
> +STDMETHODIMP AMRPacketizer::InitPlugin(IUnknown* /*IN*/ pContext)
> +{
> +    HX_RESULT retVal = HXR_OK;
> +
> +    HX_ASSERT(pContext);
> +
>
> Asserting if a NULL pContext is passed in is good, but we
> should also not crash. Check for NULL pContext and return
> HXR_INVALID_PARAMETER if it's NULL.
>
> +AMRPacketizer::GetProperties(REF(IHXValues*) pIHXValuesProperties)
> +{
> +    IHXBuffer* pBuffer = NULL;
> +    HX_RESULT res = HXR_FAIL;
> +    HX_ASSERT(m_pClassFactory);
> +    m_pClassFactory->CreateInstance(IID_IHXValues, 
> (void**)&pIHXValuesProperties);
>
> Don't assume you have m_pClassFactory - check for it being non-NULL.
>
> +     //plugin class
> +        m_pClassFactory->CreateInstance(IID_IHXBuffer, (void**)&pBuffer);
> +        if (pBuffer)
> +        {
> +            pBuffer->Set((const unsigned char*)PLUGIN_PACKETIZER_TYPE, 
> strlen(PLUGIN_PACKETIZER_TYPE)+1);
> +            pIHXValuesProperties->SetPropertyCString(PLUGIN_CLASS, 
> pBuffer);
> +            HX_RELEASE(pBuffer);
> +        }
> +        else
> +        {
> +            return HXR_OUTOFMEMORY;
> +        }
>
> Instead of doing the two steps of creating the buffer and then
> setting it into the IHXValues, just use the function
> SetCStringPropertyCCF() in common/util/pub/pckunpck.h
>
> +        //mime type
> +        m_pClassFactory->CreateInstance(IID_IHXBuffer, (void**)&pBuffer);
> +        if (pBuffer)
> +        {
> +            pBuffer->Set((const unsigned char*)AMR_NB_MIME_TYPE, 
> strlen(AMR_NB_MIME_TYPE)+1);
> + 
> pIHXValuesProperties->SetPropertyCString(PLUGIN_PACKETIZER_MIME, pBuffer);
> +            HX_RELEASE(pBuffer);
> +        }
> +        else
> +        {
> +            return HXR_OUTOFMEMORY;
> +        }
>
> Same for mime type - use SetCStringPropertyCCF.
>
> +    static const char* zm_pDescription;
> +    static const char* zm_pCopyright;
> +    static const char* zm_pMoreInfoURL;
>
> These should be "static const char* const" instead
> of "static const char*".
>
> Rest looks good.
>
> =======================================
> Eric Hyche (ehyche@real.com)
> Principal Engineer
> RealNetworks, Inc.
>
>
>>-----Original Message-----
>>From: Alok Jain [mailto:alokj@real.com]
>>Sent: Monday, March 23, 2009 1:30 AM
>>To: datatype-dev@helixcommunity.org
>>Cc: ehyche@real.com
>>Subject: CR - Implement Plugin interface in AMR
>>
>>Synopsis:
>>
>>AMRPacketizer class now implements plugin interface so as to be loadable 
>>by the producer framework.
>>Note: Umakefil still will build a static lib and in producer, a 
>>PacketizerPluginFactory will be linked
>>with this library to help in loading via the plugin finder.
>>
>>
>>
>>Overview:
>>
>>*Plugin interface implementation in AMRPacketizer class is self 
>>explanatory
>>
>>
>>
>>Files Modified:
>>
>>datatype/amr/payload/amrpacketizer.cpp
>>datatype/amr/payload/pub/amrpacketizer.h
>>
>>
>>
>>Files Added:
>>
>>datatype/amr/payload/amrpack.ver
>>
>>
>>
>>Image Size and Heap Use impact (Client -Only):
>>
>>None.
>>
>>
>>
>>Platforms and Profiles Affected:
>>
>>None.
>>
>>
>>
>>Distribution Libraries Affected:
>>
>>None.
>>
>>
>>
>>Distribution library impact and planned action:
>>
>>None.
>>
>>
>>
>>Platforms and Profiles Build Verified:
>>
>>BIF:       helixinternal
>>
>>Target:   client_encodesvc_all
>>
>>Profile:   helix-client-all-defines
>>
>>
>>
>> Thanks,
>>
>>Alok Jain
>>
>>
>>
>>
>>
>>
>>
>>
> 


_______________________________________________
Datatype-dev mailing list
Datatype-dev@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/datatype-dev
[prev in list] [next in list] [prev in thread] [next in thread]