Search code examples
comatlms-media-foundation

Is it ok to return a CComPtr from a function?


I'm writing a function which creates an IMFSample and adds a buffer to it. I would like to just return the new CComPtr to the IMFSample rather than passing a pointer to a pointer in the parameters. Is it ok to do that? Will the reference counts to the IMFSample be correct when the function exits? I've researched and haven't been able to find any examples or discussions of this usage. Here is a simplified example of the function:

CComPtr<IMFSample> getSample() {
    HRESULT hr = S_OK;
    CComPtr<IMFSample> pSample = NULL;

    hr = MFCreateSample(&pSample);
    if (hr != S_OK) {
        // pSample should be released at exception.
        throw "MFCreateSample failed";
    }

    // ...Create buffer, Add buffer, etc...

    return pSample;
}

Here I've added a longer version to help show how it simplifies my error processing by using exceptions. The rest of my program is structured using exceptions so I'm trying to isolate the "COM" parts to make them fit better with the rest as best I can.

void decodeLoop() {
    CComPtr<IMFSample> pSample;

    while (noError) {
        try {
            pSample = getNALSample(decoderPkt, MAX_SIZE);
            processSample(pSample);
            pSample = NULL;
            ...
        }
        catch (MyException& e) {
            ...
        }
    }
}

CComPtr<IMFSample> getNALSample(DecoderPkt* decoderPkt, DWORD maxBuffSize) {
    HRESULT hr = S_OK;
    CComPtr<IMFSample> pSample = nullptr;
    CComPtr<IMFMediaBuffer> pMediaBuffer = nullptr;
    byte *pBuffer = nullptr;

    // MFCreate Sample
    hr = MFCreateSample(&pSample);
    if (hr != S_OK) 
        throw MyException(hr, "getNALSample::MFCreateSample failed");

    // Get buffer
    hr = MFCreateMemoryBuffer(maxBuffSize, &pMediaBuffer);
    if (hr != S_OK) 
        throw MyException(hr, "getNALSample::MFCreateMemoryBuffer failed");

    // Set up buffer pointer
    hr = pMediaBuffer->Lock(&pBuffer, nullptr, nullptr);
    if (hr != S_OK) 
        throw MyException(hr, "getNALSample::pMediaBuffer->Lock failed");

    // Build NAL from decoder que
    hr = pMediaBuffer->SetCurrentLength(buildNAL(pBuffer, decoderPkt));
    if (hr != S_OK) 
        throw MyException(hr, "getNALSample::pMediaBuffer->SetCurrentLength failed");

    hr = pMediaBuffer->Unlock();
    if (hr != S_OK) 
        throw MyException(hr, "getNALSample::pMediaBuffer->Unlock failed");

    // Add buffer to sample
    hr = pSample->AddBuffer(pMediaBuffer);
    if (hr != S_OK) 
        throw MyException(hr, "getNALSample::pSample->AddBuffer failed");

    return pSample;
}

Thank you.


Solution

  • It works fine.

    The reason why you don't see much of this usage is because, the way it's designed, it looks like a really "private" method.

    Plus it throws instead of returning an HRESULT. Personally, I always prefer returning an HRESULT instead of throwing (even if you carry a ComPtr around), because it's easier to handle.

    Here is a (more convoluted, I admit) way of doing it that looks more "COM" (or more "Microsoft" if you will):

    HRESULT GetSample(REFIID riid, void**ppv) // you can add other parameters (in front, and leave riid and ppv last)
    {
      HRESULT hr = S_OK;
      CComPtr<IMFSample> pSample;
    
      hr = MFCreateSample(&pSample);
      if (hr != S_OK) return hr; // or if (FAILED(hr) ? depends if you handle S_FALSE an other non error cases
    
      // ...Create buffer, Add buffer, etc...
    
      return pSample->QueryInterface(riid, ppv);
    }