[prev in list] [next in list] [prev in thread] [next in thread]
List: helix-datatype-dev
Subject: RE: [datatype-dev] CR:Changes to support rmra, rmda,
From: "Eric Hyche" <ehyche () real ! com>
Date: 2009-03-30 13:15:15
Message-ID: 006101c9b139$916509e0$b42f1da0$ () com
[Download RAW message or body]
This should be checked into HEAD as well.
=======================================
Eric Hyche (ehyche@real.com)
Principal Engineer
RealNetworks, Inc.
>-----Original Message-----
>From: Varun Kathuria [mailto:vkathuria@real.com]
>Sent: Monday, March 30, 2009 2:23 AM
>To: ehyche@real.com; datatype-dev@helixcommunity.org
>Subject: CN: [datatype-dev] CR:Changes to support rmra, rmda,rdrf atom in mov file
>
>Thanks Eric,
>This has been checked into 310atlas,347atlas & 348atlas after incorporating
>your suggestions.
>
>Thanks
>Varun Kathuria
>----- Original Message -----
>From: "Eric Hyche" <ehyche@real.com>
>To: "'Varun Kathuria'" <vkathuria@real.com>;
><datatype-dev@helixcommunity.org>
>Sent: Friday, March 27, 2009 11:25 PM
>Subject: RE: [datatype-dev] CR:Changes to support rmra, rmda,rdrf atom in
>mov file
>
>
>> So if we fail to create the inner FF then we
>> should be calling MakeFileHeader(status) where
>> status is some failure code. This will cause
>> MakeFileHeader() to fall all the way down to
>> FileHeaderReady() without attempting anything else.
>>
>> Eric
>>
>> =======================================
>> Eric Hyche (ehyche@real.com)
>> Principal Engineer
>> RealNetworks, Inc.
>>
>>
>>>-----Original Message-----
>>>From: Varun Kathuria [mailto:vkathuria@real.com]
>>>Sent: Friday, March 27, 2009 1:37 PM
>>>To: ehyche@real.com; datatype-dev@helixcommunity.org
>>>Subject: Re: [datatype-dev] CR:Changes to support rmra, rmda,rdrf atom in
>>>mov file
>>>
>>>Hi Eric,
>>>
>>>I am not passing any failure code but in case of rmra atom it will fail in
>>>
>>> status = m_TrackManager.ReadyTracks(bIgnoreHintTracks,
>>> m_bViewSourceRequest);
>>>
>>>Here status will fail in AtomReady().
>>>
>>>So due to this in :
>>>
>>> retVal = MakeFileHeader(status);
>>>
>>>Failure code has passed to status.
>>>
>>>Thanks
>>>Varun Kathuria
>>>
>>>----- Original Message -----
>>>From: "Eric Hyche" <ehyche@real.com>
>>>To: "'Varun Kathuria'" <vkathuria@real.com>;
>>><datatype-dev@helixcommunity.org>
>>>Sent: Friday, March 27, 2009 6:25 PM
>>>Subject: RE: [datatype-dev] CR:Changes to support rmra, rmda,rdrf atom in
>>>mov file
>>>
>>>
>>>>>> Make sure you handle the failure case correctly. Right now
>>>>>> if we fail to create the file format for some reason,
>>>>>> then the outerFF will hang, since it will just return
>>>>>> a failure code in AtomReady(), but it will never make
>>>>>> a call to FileHeaderReady(). This will hang the HXFileSource
>>>>>> which is waiting on the FileHeaderReady() call.
>>>>>
>>>>>Varun: For handling failure case correctly.
>>>>> I have replaced
>>>>> + return retVal
>>>>> with
>>>>> + if(SUCCEDDED(retVal))
>>>>> + {
>>>>> + return retVal
>>>>> + }
>>>>>Now if we fail to create fileformat due to some reason then it will not
>>>>>return. It will call MakeFileHeader() in AtomReady() which will call
>>>>>FileHeaderReady() with in itself & it will not hang filesource.
>>>>>
>>>>
>>>> I assume you are passing a failure code into MakeFileHeader(), correct?
>>>>
>>>> If so, then this looks good for checkin.
>>>>
>>>> Eric
>>>>
>>>> =======================================
>>>> Eric Hyche (ehyche@real.com)
>>>> Principal Engineer
>>>> RealNetworks, Inc.
>>>>
>>>>>-----Original Message-----
>>>>>From: Varun Kathuria [mailto:vkathuria@real.com]
>>>>>Sent: Friday, March 27, 2009 7:11 AM
>>>>>To: ehyche@real.com; datatype-dev@helixcommunity.org
>>>>>Subject: Re: [datatype-dev] CR:Changes to support rmra, rmda,rdrf atom
>>>>>in
>>>>>mov file
>>>>>
>>>>>Hi Eric,
>>>>>
>>>>>Please find the attached updated diff.
>>>>>& see replies inline.
>>>>>
>>>>>Thanks
>>>>>Varun Kathuria
>>>>>----- Original Message -----
>>>>>From: "Eric Hyche" <ehyche@real.com>
>>>>>To: "'Varun Kathuria'" <vkathuria@real.com>;
>>>>><datatype-dev@helixcommunity.org>
>>>>>Sent: Thursday, March 26, 2009 8:41 PM
>>>>>Subject: RE: [datatype-dev] CR:Changes to support rmra, rmda,rdrf atom
>>>>>in
>>>>>mov file
>>>>>
>>>>>
>>>>>> Varun,
>>>>>>
>>>>>> Here are my comments:
>>>>>>
>>>>>> STDMETHODIMP CQTFileFormat::GetPacket(UINT16 unStreamNumber)
>>>>>> {
>>>>>> + if(m_pRedirectFFObject)
>>>>>> + {
>>>>>> + return m_pRedirectFFObject->GetPacket(unStreamNumber);
>>>>>> + }
>>>>>> +
>>>>>>
>>>>>> You should set the state, like this:
>>>>>>
>>>>>> STDMETHODIMP CQTFileFormat::GetPacket(UINT16 unStreamNumber)
>>>>>> {
>>>>>> + if(m_pRedirectFFObject)
>>>>>> + {
>>>>>> + m_state = QTFF_GetPacket;
>>>>>> + return m_pRedirectFFObject->GetPacket(unStreamNumber);
>>>>>> + }
>>>>>> +
>>>>>>
>>>>>> STDMETHODIMP CQTFileFormat::Seek(ULONG32 ulOffset)
>>>>>> {
>>>>>> + if(m_pRedirectFFObject)
>>>>>> + {
>>>>>> + return m_pRedirectFFObject->Seek(ulOffset);
>>>>>> + }
>>>>>> +
>>>>>>
>>>>>> Same thing here - set the state to QTFF_SeekPending
>>>>>> before calling m_pRedirectFFObject->Seek(ulOffset).
>>>>>>
>>>>>> STDMETHODIMP CQTFileFormat::SeekDone(HX_RESULT status)
>>>>>> {
>>>>>> + if(m_State == QTFF_Atomize)
>>>>>> + {
>>>>>> + return m_pFFResponse->SeekDone(status);
>>>>>> + }
>>>>>>
>>>>>> This is not correct. You are assuming here that
>>>>>> the outer FF always stays in the state QTFF_Atomize.
>>>>>> It should not. The state of the outer FF should be
>>>>>> updating to the correct states even though it is proxying
>>>>>> calls to the inner FF. The states should be:
>>>>>>
>>>>>> - Set to QTFF_Offline upon creation of CQTFileFormat until
>>>>>> InitFileFormat is called.
>>>>>> - Set to QTFF_Init upon call to InitFileFormat and then
>>>>>> set to QTFF_Ready when IHXFormatResponse::InitDone() is called.
>>>>>> - Set to QTFF_Atomize when GetFileHeader() is called and
>>>>>> then set back to QTFF_Ready() right before FileHeaderReady() is
>>>>>> called.
>>>>>> - Set to QTFF_GetPacket when GetPacket() is called and
>>>>>> returned to QTFF_Ready right before PacketReady() is called.
>>>>>> - Set the QTFF_SeekPending when Seek() is called
>>>>>> and then reset to QTFF_Ready right before SeekDone() is called.
>>>>>>
>>>>>> So the code above should be:
>>>>>>
>>>>>> STDMETHODIMP CQTFileFormat::SeekDone(HX_RESULT status)
>>>>>> {
>>>>>> + if (m_pRedirectFFObject && m_State == QTFF_SeekPending)
>>>>>> + {
>>>>>> + m_State = QTFF_Ready;
>>>>>> + return m_pFFResponse->SeekDone(status);
>>>>>> + }
>>>>>>
>>>>>>
>>>>>>
>>>>>> }
>>>>>> -
>>>>>> + HX_RELEASE(m_pRedirectFFObject);
>>>>>> m_TrackManager.CloseTracks();
>>>>>> You need to call Close() on the m_pRedirectFFObject
>>>>>> before releasing it.
>>>>>>
>>>>>>
>>>>>> + status =
>>>>>> m_MovieInfo.Init(pRootAtom->FindPresentChild(QT_moov),
>>>>>> &m_TrackManager);
>>>>>> Don't think you need to call pRootAtom->FindPresentChild(QT_moov)
>>>>>> again here. You already have the moov atom in pMoovAtom
>>>>>> from above.
>>>>>
>>>>>Varun: There is no need of calling pRootAtom->FindPresentChild(QT_moov)
>>>>>again.Replaced it with pMoovAtom
>>>>>
>>>>>> + if(purl.IsRelativeURL())
>>>>>> + {
>>>>>> + //If the URL is relative, then this
>>>>>> code
>>>>>> changes it to absolute URL.
>>>>>> + const char *poriginal_url = NULL;
>>>>>> + m_pRequest->GetURL(poriginal_url);
>>>>>> + const char *prel =
>>>>>> strrchr(poriginal_url,'/');
>>>>>> + szURL = poriginal_url;
>>>>>> + szURL =
>>>>>> szURL.Left(prel-poriginal_url+1);
>>>>>> + szURL += pszReferenceURL;
>>>>>> + }
>>>>>>
>>>>>> Has this code been tested? Have you found a
>>>>>> .mov with a relative URL?
>>>>>
>>>>>Varun: The code for URL is tested for both relative & absolute URL.
>>>>>
>>>>>> + retVal =
>>>>>> CreateNewFileFormatObject(szURL.GetBuffer(0));
>>>>>>
>>>>>> Why szURL.GetBuffer()? Don't you just need the
>>>>>> const char* that this CHXString represents? If so,
>>>>>> then you should just cast the CHXString to const char*, like this:
>>>>>>
>>>>>> + retVal = CreateNewFileFormatObject((const
>>>>>> char*) szURL);
>>>>>>
>>>>>>
>>>>>> + status =
>>>>>> m_MovieInfo.Init(pRootAtom->FindPresentChild(QT_moov),
>>>>>> &m_TrackManager);
>>>>>> + if (SUCCEEDED(status))
>>>>>> + {
>>>>>> + retVal = HXR_FAIL;
>>>>>> + // Get the Reference Url
>>>>>> + const char* pszReferenceURL = NULL;
>>>>>> + pszReferenceURL = m_MovieInfo.GetRefURL();
>>>>>> + if(pszReferenceURL)
>>>>>> + {
>>>>>> + CHXString szURL="";
>>>>>> + CHXURL purl(pszReferenceURL,m_pContext);
>>>>>> + if(purl.IsRelativeURL())
>>>>>> + {
>>>>>> + //If the URL is relative, then this
>>>>>> code
>>>>>> changes it to absolute URL.
>>>>>> + const char *poriginal_url = NULL;
>>>>>> + m_pRequest->GetURL(poriginal_url);
>>>>>> + const char *prel =
>>>>>> strrchr(poriginal_url,'/');
>>>>>> + szURL = poriginal_url;
>>>>>> + szURL =
>>>>>> szURL.Left(prel-poriginal_url+1);
>>>>>> + szURL += pszReferenceURL;
>>>>>> + }
>>>>>> + else
>>>>>> + {
>>>>>> + szURL = pszReferenceURL;
>>>>>> + }
>>>>>> + retVal =
>>>>>> CreateNewFileFormatObject(szURL.GetBuffer(0));
>>>>>> + }
>>>>>> + }
>>>>>> + return retVal;
>>>>>>
>>>>>> Make sure you handle the failure case correctly. Right now
>>>>>> if we fail to create the file format for some reason,
>>>>>> then the outerFF will hang, since it will just return
>>>>>> a failure code in AtomReady(), but it will never make
>>>>>> a call to FileHeaderReady(). This will hang the HXFileSource
>>>>>> which is waiting on the FileHeaderReady() call.
>>>>>
>>>>>Varun: For handling failure case correctly.
>>>>> I have replaced
>>>>> + return retVal
>>>>> with
>>>>> + if(SUCCEDDED(retVal))
>>>>> + {
>>>>> + return retVal
>>>>> + }
>>>>>Now if we fail to create fileformat due to some reason then it will not
>>>>>return. It will call MakeFileHeader() in AtomReady() which will call
>>>>>FileHeaderReady() with in itself & it will not hang filesource.
>>>>>
>>>>>> STDMETHOD(GetSupportedPacketFormats) (THIS_
>>>>>> REF(const char**) pFormats);
>>>>>>
>>>>>> STDMETHOD(SetPacketFormat) (THIS_
>>>>>> const char* pFormat);
>>>>>>
>>>>>> STDMETHOD(ConvertFileOffsetToDur) (THIS_
>>>>>> UINT32 /*IN*/ ulLastReadOffset,
>>>>>> UINT32 /*IN*/
>>>>>> ulCompleteFileSize,
>>>>>> REF(UINT32) /*OUT*/
>>>>>> ulREFDuration);
>>>>>>
>>>>>> STDMETHOD(GetFileDuration) (THIS_
>>>>>> UINT32 /*IN*/
>>>>>> ulCompleteFileSize,
>>>>>> REF(UINT32) /*OUT*/ ulREFDur);
>>>>>>
>>>>>> These methods also need to be redirected to the inner FF.
>>>>>
>>>>>> + pRequest->SetURL((const char*)purl); // Set the reference URL
>>>>>> + pRequestHandler->SetRequest(pRequest);
>>>>>>
>>>>>> You probably also need to take the request headers
>>>>>> from the request passed to the outer FF and copy
>>>>>> them into this request that is passed to the inner FF.
>>>>>
>>>>>Varun: I have taken request headers from outer FF & passes them into
>>>>>inner
>>>>>file format request.
>>>>>
>>>>>> + if (FAILED(pPlugin->InitPlugin((IUnknown*)
>>>>>> (IHXStreamSource*)this)))
>>>>>>
>>>>>> This looks like a cut-n-paste error. CQTFileFormat does not implement
>>>>>> IHXStreamSource, so this is not correct. The outer FF should
>>>>>> just pass along the context it received in InitPlugin(). So this
>>>>>> should just be:
>>>>>>
>>>>>> + if (FAILED(pPlugin->InitPlugin(m_pContext)))
>>>>>>
>>>>>> + theErr =
>>>>>> m_pRedirectFFObject->InitFileFormat(pRequest,this,pFileObject);
>>>>>>
>>>>>> You need to cast the "this" pointer to (IHXFormatResponse*) here.
>>>>>>
>>>>>> + STDMETHODIMP CQTFileFormat::PacketReady(THIS_
>>>>>> + HX_RESULT status,
>>>>>> + IHXPacket* pPacket)
>>>>>> + {
>>>>>> + return m_pFFResponse->PacketReady(status,pPacket);
>>>>>> + }
>>>>>> +
>>>>>> + STDMETHODIMP CQTFileFormat::FileHeaderReady (THIS_
>>>>>> + HX_RESULT status,
>>>>>> + IHXValues* pHeader)
>>>>>> + {
>>>>>> + return m_pFFResponse->FileHeaderReady(status,pHeader);
>>>>>> + }
>>>>>> +
>>>>>> + STDMETHODIMP CQTFileFormat::StreamHeaderReady (THIS_
>>>>>> + HX_RESULT status,
>>>>>> + IHXValues* pHeader)
>>>>>> + {
>>>>>> + return m_pFFResponse->StreamHeaderReady(status,pHeader);
>>>>>> + }
>>>>>> +
>>>>>> + STDMETHODIMP CQTFileFormat::StreamDone(THIS_
>>>>>> + UINT16 unStreamNumber)
>>>>>> + {
>>>>>> + return m_pFFResponse->StreamDone(unStreamNumber);
>>>>>> + }
>>>>>> +
>>>>>>
>>>>>> In all of these the outer FF needs to reset m_state back to
>>>>>> the appropriate state before making the IHXFormatResponse call.
>>>>>>
>>>>>> + * IHXFormatResponse methods
>>>>>> + */
>>>>>> +
>>>>>> + STDMETHOD(PacketReady) (THIS_
>>>>>> + HX_RESULT status,
>>>>>> + IHXPacket* pPacket);
>>>>>> +
>>>>>> + STDMETHOD(FileHeaderReady) (THIS_
>>>>>> + HX_RESULT status,
>>>>>> + IHXValues* pHeader) ;
>>>>>> +
>>>>>> + STDMETHOD(StreamHeaderReady) (THIS_
>>>>>> + HX_RESULT status,
>>>>>> + IHXValues* pHeader) ;
>>>>>> +
>>>>>> + STDMETHOD(StreamDone) (THIS_
>>>>>> + UINT16 unStreamNumber) ;
>>>>>> +
>>>>>> + /*
>>>>>>
>>>>>> Please include InitDone() and SeekDone() but comment
>>>>>> them out and include a comment that these methods
>>>>>> are duplicated in IHXFileResponse, which explains
>>>>>> why they would be commented out.
>>>>>>
>>>>>> + HX_RESULT CreateNewFileFormatObject(char* purl);
>>>>>>
>>>>>> This should be const char* instead of char*.
>>>>>>
>>>>>>
>>>>>> =======================================
>>>>>> Eric Hyche (ehyche@real.com)
>>>>>> Principal Engineer
>>>>>> RealNetworks, Inc.
>>>>>>
>>>>>>
>>>>
>>
_______________________________________________
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]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic