RE: Get abstrct information of media stream throughdtdriver (hxclientkit)

This looks good to me.

=============================================
Eric Hyche ([email protected])
Technical Lead
RealNetworks, Inc.  

> -----Original Message-----
> From: Zhao, Halley [mailto:[email protected]] 
> Sent: Wednesday, May 21, 2008 5:48 AM
> To: [email protected]; [email protected]
> Cc: Lynch, Rusty
> Subject: RE: [Helix-client-dev] Get abstrct information of 
> media stream throughdtdriver (hxclientkit)
> 
> Hi Eric:
> Here is the patch following your feedback.
> Thanks & BR.
> 
> 
> >-----Original Message-----
> >From: Eric Hyche [mailto:[email protected]]
> >Sent: 2008521 1:07
> >To: Zhao, Halley; [email protected]
> >Cc: Lynch, Rusty
> >Subject: RE: [Helix-client-dev] Get abstrct information of 
> media stream
> >throughdtdriver (hxclientkit)
> >
> >
> >> -----Original Message-----
> >> From: Zhao, Halley [mailto:[email protected]]
> >> Sent: Monday, May 19, 2008 10:39 PM
> >> To: [email protected]; [email protected]
> >> Cc: Lynch, Rusty
> >> Subject: RE: [Helix-client-dev] Get abstrct information of
> >> media stream throughdtdriver (hxclientkit)
> >>
> >> >No. When you use PROCESSHEADERS_OPTION_NAME, you should
> >> >only use that option and NOT set  DECODEAUDIO_OPTION_NAME
> >> >and DECODEVIDEO_OPTION_NAME.
> >> Really? I will try it.
> >> As far as I know, when I used DECODEVIDEO_OPTION_NAME,
> >> while not DECODEAUDIO_OPTION_NAME (just as thumbnail does). I
> >> willnot get callback on OnStreamHeader() for audio.
> >>
> >
> >Let me be clear: when you use "ProcessHeadersOnly" = 1 in 
> the options,
> >you should NOT set DECODEAUDIO_OPTION_NAME and you
> >should NOT set DECODEVIDEO_OPTION_NAME. When you set
> >"ProcessHeadersOnly" = 1, that should be the ONLY option you set.
> >
> >Eric
> >
> >>
> >>
> >> >-----Original Message-----
> >> >From: Eric Hyche [mailto:[email protected]]
> >> >Sent: 2008520 10:25
> >> >To: Zhao, Halley; [email protected]
> >> >Cc: Lynch, Rusty
> >> >Subject: RE: [Helix-client-dev] Get abstrct information of
> >> media stream
> >> >throughdtdriver (hxclientkit)
> >> >
> >> >
> >> >Halley,
> >> >
> >> >comments inline below...
> >> >
> >> >> -----Original Message-----
> >> >> From: Zhao, Halley [mailto:[email protected]]
> >> >> Sent: Monday, May 19, 2008 10:01 PM
> >> >> To: [email protected]; [email protected]
> >> >> Cc: Lynch, Rusty
> >> >> Subject: RE: [Helix-client-dev] Get abstrct information of
> >> >> media stream throughdtdriver (hxclientkit)
> >> >>
> >> >> Agree, and with 2 following-up questions:
> >> >> 1. for headerOnly, I will just use 3 options:
> >> >>    DECODEAUDIO_OPTION_NAME, DECODEVIDEO_OPTION_NAME,
> >> >> PROCESSHEADERS_OPTION_NAME.
> >> >> Is it correct?
> >> >>
> >> >
> >> >No. When you use PROCESSHEADERS_OPTION_NAME, you should
> >> >only use that option and NOT set  DECODEAUDIO_OPTION_NAME
> >> >and DECODEVIDEO_OPTION_NAME.
> >> >
> >> >> 2. for some common properties in both audio and video, ( I
> >> >> think there may be even more than one audio/video stream).
> >> >> So I'd like add integer variables in class CSourceInput,
> >> >> for example: m_VideoIndex, m_AudioIndex. The variables will
> >> >> be reset to 0 in OnFileHeader(), and increase respectively
> >> >> (for audio or video) in OnStreamHeader().
> >> >> And we will combine this 'stream index' with the 
> properties
> >> >> name, for example: "video0:width, 320", or
> >> "audio1:AvgBitRate:32000"
> >> >>
> >> >
> >> >However, you want to design the dbus interface is fine
> >> with me. This
> >> >looks like it would fix the problem of common 
> properties between
> >> >streams.
> >> >
> >> >Eric
> >> >
> >> >
> >> >> >-----Original Message-----
> >> >> >From: Eric Hyche [mailto:[email protected]]
> >> >> >Sent: 2008519 23:49
> >> >> >To: Zhao, Halley; [email protected]
> >> >> >Cc: Lynch, Rusty
> >> >> >Subject: RE: [Helix-client-dev] Get abstrct 
> information of
> >> >> media stream
> >> >> >throughdtdriver (hxclientkit)
> >> >> >
> >> >> >
> >> >> >Halley,
> >> >> >
> >> >> >My comments:
> >> >> >
> >> >> >@@ -190,7 +222,11 @@
> >> >> >     	// 
> pOptions->SetPropertyULONG32("StartTime", 10);
> >> >> >     	pOptions->SetPropertyULONG32("StartTime",
> >startTime);
> >> >> >     	
> pOptions->SetPropertyULONG32(DECODE_OPTION_NAME, 1);
> >> >> >-    	
> //pOptions->SetPropertyULONG32("VideoThumbnail", 1);
> >> >> >+        if(headerOnly)
> >> >> >+        {
> >> >> >+
> >> >> pOptions->SetPropertyULONG32(DECODEAUDIO_OPTION_NAME, 1);
> >> >> >+
> >> pOptions->SetPropertyULONG32("ProcessHeadersOnly",
> >> >1);
> >> >> >+        }
> >> >> >
> >> >> >If you are setting "ProcessHeadersOnly", then you
> >> should NOT be
> >> >> >setting any of the other decode-related options. The
> >> reason is that
> >> >> >if you are decoding, then some of the file header
> >> >> properties are not
> >> >> >provided, and also that the stream header properties
> >> are different
> >> >> >when decoding.
> >> >> >
> >> >> >@@ -650,18 +782,22 @@
> >> >> >     //printf ("CSourceInput::OnStreamHeader\n");
> >> >> >     IHXBuffer* pMime = NULL;
> >> >> >     pValues->GetPropertyCString("MimeType", pMime);
> >> >> >-    //printf("Stream MimeType: %s\n",
> >pMime->GetBuffer());
> >> >> >+    if( '\0' == g_sMimeType[0])
> >> >> >+    {
> >> >> >+        strcat( g_sMimeType, pMime->GetBuffer() );
> >> >> >+    }
> >> >> >+    else
> >> >> >+    {
> >> >> >+        strcat(g_sMimeType, ":");
> >> >> >+        strcat( g_sMimeType, pMime->GetBuffer() );
> >> >> >+    }
> >> >> >
> >> >> >This runs the risk of buffer overrun since g_sMimeType is
> >> >> a fixed-length
> >> >> >buffer of length 256. You should use SafeSprintf or
> >SafeStrCat
> >> >> >defined in common/runtime/pub/safestring.h instead.
> >> >> >
> >> >> >@@ -80,13 +80,16 @@
> >> >> >     IHXDataTypeDriverResponse* m_pDTDResponse;
> >> >> >     IHXSourceInput* m_pSourceInput;
> >> >> >
> >> >> >+    bool m_bHeaderOnly ;
> >> >> >
> >> >> >We should use HX_BOOL here instead.
> >> >> >
> >> >> >
> >> >> >-/*
> >> >> >-    UINT32 ulChan = 0;
> >> >> >-    pValues->GetPropertyULONG32("Channels", ulChan);
> >> >> >-    if (ulChan == 2)
> >> >> >+    if( NULL != pValues)
> >> >> >     {
> >> >> >-	printf("Stereo\n");
> >> >> >+        DumpMediaInfo(pValues);
> >> >> >     }
> >> >> >-*/
> >> >> >-    //UINT32 ulSamplsPerSec = 0;
> >> >> >+
> >> >> >
> >> >> >This looks like you are passing both stream headers into
> >> >> DumpMediaInfo().
> >> >> >Both audio and video headers will have several properties
> >> >> in common, such
> >> >> >as "AvgBitRate". How will you distinguish between the
> >> >> "AvgBitRate" of
> >> >> >a video stream and an "AvgBitRate" of an audio stream?
> >> >> >
> >> >> >Rest looks good.
> >> >> >
> >> >> >Eric
> >> >> >=============================================
> >> >> >Eric Hyche ([email protected])
> >> >> >Technical Lead
> >> >> >RealNetworks, Inc.
> >> >> >
> >> >> >> -----Original Message-----
> >> >> >> From: Zhao, Halley [mailto:[email protected]]
> >> >> >> Sent: Tuesday, May 13, 2008 8:24 PM
> >> >> >> To: [email protected]; 
> [email protected]
> >> >> >> Cc: Lynch, Rusty
> >> >> >> Subject: RE: [Helix-client-dev] Get abstrct 
> information of
> >> >> >> media stream throughdtdriver (hxclientkit)
> >> >> >>
> >> >> >> Sorry for mistake.
> >> >> >>
> >> >> >> Attach the patch file.
> >> >> >>
> >> >> >>
> >> >> >> >-----Original Message-----
> >> >> >> >From: Eric Hyche [mailto:[email protected]]
> >> >> >> >Sent: 2008513 23:45
> >> >> >> >To: Zhao, Halley;
> >[email protected]
> >> >> >> >Cc: Lynch, Rusty
> >> >> >> >Subject: RE: [Helix-client-dev] Get abstrct
> >> information of
> >> >> >> media stream
> >> >> >> >throughdtdriver (hxclientkit)
> >> >> >> >
> >> >> >> >
> >> >> >> >> Files Attached:
> >> >> >> >>
> >> >> >> >>     hxclientkit-mediaAbstractInfo.diff.txt
> >> >> >> >>
> >> >> >> >
> >> >> >> >I didn't see any such file attached to the
> >> >> >> >CR email.
> >> >> >> >
> >> >> >> >Eric
> >> >> >> >
> >> >> >> >=============================================
> >> >> >> >Eric Hyche ([email protected])
> >> >> >> >Technical Lead
> >> >> >> >RealNetworks, Inc.
> >> >> >> >
> >> >> >> >> -----Original Message-----
> >> >> >> >> From: [email protected]
> >> >> >> >>
> >[mailto:[email protected]] On
> >> >> >> >> Behalf Of Zhao, Halley
> >> >> >> >> Sent: Tuesday, May 13, 2008 5:56 AM
> >> >> >> >> To: [email protected]
> >> >> >> >> Cc: Lynch, Rusty
> >> >> >> >> Subject: [Helix-client-dev] Get abstrct
> >information of
> >> >media
> >> >> >> >> stream throughdtdriver (hxclientkit)
> >> >> >> >>
> >> >> >> >> Synopsis:
> >> >> >> >>
> >> >> >> >>     Get abstrct information of media 
> stream through
> >> >dtdriver
> >> >> >> >>
> >> >> >> >> Overview:
> >> >> >> >>
> >> >> >> >>     use dtdriver plugin to parse a media stream's
> >> >> file/stream
> >> >> >> >> header to get some information,
> >> >> >> >>
> >> >> >> >>     for example: Title, width, hegith, Copyright,
> >etc.
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >> Files Added:
> >> >> >> >>
> >> >> >> >>        No file added
> >> >> >> >>
> >> >> >> >> Files Modified:
> >> >> >> >>
> >> >> >> >>     player/hxclientkit/pub/HXClientCFuncs.h
> >> >> >> >>
> >> >> >> >>     player/hxclientkit/pub/HXClientCallbacks.h
> >> >> >> >>
> >> >> >> >>
> >> >>
> >>player/hxclientkit/src/platform/unix/CHXClientThumbnailUnix.h
> >> >> >> >>
> >> >> >> >>         function and variables declaration
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >>
> >> 
> >player/hxclientkit/src/platform/unix/CHXClientThumbnailUnix.cpp
> >> >> >> >>
> >> >> >> >>         function implementation
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >> Image Size and Heap Use impact (Client -Only):
> >> >> >> >>
> >> >> >> >>     little
> >> >> >> >>
> >> >> >> >> Platforms and Profiles Affected:
> >> >> >> >>
> >> >> >> >>     platform:   linux-2.2-libc6-gcc32-i586
> >> >> >> >>
> >> >> >> >>     profile:    helix-client-all-defines
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >> Distribution Libraries Affected:
> >> >> >> >>
> >> >> >> >>     <libhxclient.a>
> >> >> >> >>
> >> >> >> >> Distribution library impact and planned action:
> >> >> >> >>
> >> >> >> >>     <None>
> >> >> >> >>
> >> >> >> >> Platforms and Profiles Build Verified:
> >> >> >> >>
> >> >> >> >>     Set BIF branch  -> hxdbus_3_1_0_atlas
> >> >> >> >>
> >> >> >> >>     Set Target(s)   -> dbus_server_with_video
> >> >> >> >>
> >> >> >> >>     Set Profile     -> helix-client-all-defines
> >> >> >> >>
> >> >> >> >>     System ID       -> linux-2.2-libc6-gcc32-i586
> >> >> >> >>
> >> >> >> >> Branch:
> >> >> >> >>
> >> >> >> >>     HEAD, hxclient_3_1_0_atlas
> >> >> >> >>
> >> >> >> >> Copyright assignment: <MUST be one of the 
> following
> >> >> statements >
> >> >> >> >>
> >> >> >> >>    2.      Intel has signed and delivered a Joint
> >> >> >> Copyright Assignment
> >> >> >> >>
> >> >> >> >>            to RealNetworks, and received
> >> >> acknowledgment that the
> >> >> >> >>
> >> >> >> >>            agreement was received.
> >> >> >> >>
> >> >> >> >> Files Attached:
> >> >> >> >>
> >> >> >> >>     hxclientkit-mediaAbstractInfo.diff.txt
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >> ZHAO, Halley (Aihua)
> >> >> >> >>
> >> >> >> >> Email: [email protected]
> >> ><mailto:[email protected]>
> >> >> >> >>
> >> >> >> >> Tel: +86(21)61166476
> >> >> >> >>
> >> >> >> >> iNet: 8821-6476
> >> >> >> >>
> >> >> >> >> SSG/OTC/UMD
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >>
> >> >> >>
> >> >>
> >> >>
> >>
> >>
> 
> 


_______________________________________________
Helix-client-dev mailing list
[email protected]
http://lists.helixcommunity.org/mailman/listinfo/helix-client-dev