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

List:       helix-datatype-dev
Subject:    RE: [datatype-dev] CR: Thumbnail generation improvement updates
From:       "Eric Hyche" <ehyche () real ! com>
Date:       2008-10-16 13:52:37
Message-ID: 009601c92f96$73e001b0$5ba00510$ () com
[Download RAW message or body]

>(I¡¯ve found that it has not been shutdown gracefully, so I¡¯ve added the line which calls
>OnTermination() in OnPacket())
>

Ok - that's what I thought - that we needed to call OnTermination() ourselves.

This looks good for checkin.

Eric

=======================================
Eric Hyche (ehyche@real.com)
Principal Engineer
RealNetworks, Inc.


>-----Original Message-----
>From: ÃÖÇÏ¿µ(Hayoung Choi) [mailto:hayoung.choi@ap.real.com]
>Sent: Thursday, October 16, 2008 8:32 AM
>To: ehyche@real.com; datatype-dev@lists.helixcommunity.org
>Subject: RE: [datatype-dev] CR: Thumbnail generation improvement updates
>
>Eric,
>
>
>
>Here is the code flow.
>
>
>
>Several packets come into the OnPacket() in CVideoSourceHandler from CSourceSplitter.
>
>The first packet is proceeded through VideoRenderer and CStubSite, and then it is passed to the
>OnVideoBlt() in CVideoSourceHandler.
>
>If it is not an I420 format, then OnVideoBlt() returns HXR_UNSUPPORTED_VIDEO not passing it to the
>CProcessResponse.
>
>So, the packet will be ignored. The next packet might be passed into the OnVideoBlt(), and it also
>will be ignored.
>
>(I¡¯ve found that it has not been shutdown gracefully, so I¡¯ve added the line which calls
>OnTermination() in OnPacket())
>
>
>
>However, some packets will be passed to OnPacket() in CVideoSourceHandler from CSourceSplitter.
>
>At that time, *OnTermination()* will be called in OnPacket() in CVideoSourceHandler.
>
>Please refer to below red line. Is it correct?
>
>
>
>@@ -438,8 +454,16 @@
>
>     HXLOGL4(HXLOG_DTDR, "CVideoSourceHandler[%p]::OnPacket(status=0x%08x,)", this, status);
>
>     HX_RESULT retVal = HXR_OK;
>
>
>
>+    if (m_bDoResizingVideo && !m_bValidResizingInputFormat)
>
>+    {
>
>+        OnTermination(HXR_UNSUPPORTED_VIDEO);
>
>+        return HXR_UNSUPPORTED_VIDEO;
>
>+    }
>
>+
>
>
>
>@@ -681,13 +720,56 @@
>
>     UINT32 currentTime = 0;
>
>     m_pRenderTimeLine->GetTimeLineValue(currentTime);
>
>
>
>+    if (m_bDoResizingVideo && !m_bValidResizingInputFormat)
>
>+    {
>
>+        return HXR_UNSUPPORTED_VIDEO;
>
>+    }
>
>+
>
>       if (m_bFirstWrite)
>
>       {
>
>-            if (m_bDoConversion)
>
>+       // Set input and output width/height size for resizing
>
>+       m_ulInputWidth = pHeader->biWidth;
>
>+       m_ulInputHeight = pHeader->biHeight;
>
>+
>
>+       if (m_bDoResizingVideo)
>
>+       {
>
>+           if (pHeader->biCompression != HX_I420)
>
>+           {
>
>+               m_bValidResizingInputFormat = FALSE;
>
>+               return HXR_UNSUPPORTED_VIDEO;
>
>+           }
>
>+
>
>
>
>Thanks,
>
>Hayoung
>
>-----Original Message-----
>From: Eric Hyche [mailto:ehyche@real.com]
>Sent: Thursday, October 16, 2008 1:08 AM
>To: ÃÖÇÏ¿µ(Hayoung Choi); datatype-dev@lists.helixcommunity.org
>Subject: RE: [datatype-dev] CR: Thumbnail generation improvement updates
>
>
>
>So what is the code flow when OnVideoBlt() returns
>
>HXR_UNSUPPORTED_VIDEO? Where and when does the decoder
>
>source handler call OnTermination() to the rest
>
>of the source handler stack?
>
>
>
>Eric
>
>
>
>=======================================
>
>Eric Hyche (ehyche@real.com)
>
>Principal Engineer
>
>RealNetworks, Inc.
>
>
>
>
>
>>-----Original Message-----
>
>>From: ÃÖÇÏ¿µ(Hayoung Choi) [mailto:hayoung.choi@ap.real.com]
>
>>Sent: Tuesday, October 14, 2008 5:44 AM
>
>>To: ehyche@real.com; datatype-dev@lists.helixcommunity.org
>
>>Subject: RE: [datatype-dev] CR: Thumbnail generation improvement updates
>
>>
>
>>Eric,
>
>>
>
>>I have handled the error to return HXR_UNSUPPORTED_VIDEO error when it comes with other than I420
>
>>input format like below.
>
>>As a result of test, it looks running well when it returns HXR_UNSUPPORTED_VIDEO error, then
>
>>OnTermination() is called and be shutdowned properly.
>
>>
>
>>[vdecoder.h]
>
>>+    HXBOOL                m_bValidResizingInputFormat;
>
>>
>
>>[vdecoder.cpp]
>
>>@@ -681,13 +714,56 @@
>
>>     UINT32 currentTime = 0;
>
>>     m_pRenderTimeLine->GetTimeLineValue(currentTime);
>
>>
>
>>+    if (m_bDoResizingVideo && !m_bValidResizingInputFormat)
>
>>+    {
>
>>+        return HXR_UNSUPPORTED_VIDEO;
>
>>+    }
>
>>
>
>>+       if (m_bDoResizingVideo)
>
>>+       {
>
>>+           if (pHeader->biCompression != HX_I420)
>
>>+           {
>
>>+               m_bValidResizingInputFormat = FALSE;
>
>>+               return HXR_UNSUPPORTED_VIDEO;
>
>>+           }
>
>>
>
>>Thanks,
>
>>Hayoung
>
>>-----Original Message-----
>
>>From: Eric Hyche [mailto:ehyche@real.com]
>
>>Sent: Friday, October 10, 2008 11:26 PM
>
>>To: ÃÖÇÏ¿µ(Hayoung Choi); datatype-dev@lists.helixcommunity.org
>
>>Subject: RE: [datatype-dev] CR: Thumbnail generation improvement updates
>
>>
>
>>>[Hayoung] I see. Then I¡¯ll proceed it to return HXR_FAIL error when it comes with other than I420
>
>>>input frame.
>
>>>
>
>>
>
>>Make sure that if/when we fail, we properly call
>
>>OnTermination() at some point so that we shutdown
>
>>correctly and not hang the dtdriver pipeline.
>
>>
>
>>Eric
>
>>
>
>>=======================================
>
>>Eric Hyche (ehyche@real.com)
>
>>Principal Engineer
>
>>RealNetworks, Inc.
>
>>
>
>>
>
>>>-----Original Message-----
>
>>>From: ÃÖÇÏ¿µ(Hayoung Choi) [mailto:hayoung.choi@ap.real.com]
>
>>>Sent: Friday, October 10, 2008 5:33 AM
>
>>>To: ehyche@real.com; datatype-dev@lists.helixcommunity.org
>
>>>Subject: RE: [datatype-dev] CR: Thumbnail generation improvement updates
>
>>>
>
>>>Eric,
>
>>>
>
>>>
>
>>>
>
>>>Thank you for the comments.
>
>>>
>
>>>My answers are inline below.
>
>>>
>
>>>
>
>>>
>
>>>Thanks,
>
>>>
>
>>>Hayoung
>
>>>
>
>>>-----Original Message-----
>
>>>From: Eric Hyche [mailto:ehyche@real.com]
>
>>>Sent: Wednesday, October 08, 2008 11:28 PM
>
>>>To: ÃÖÇÏ¿µ(Hayoung Choi); datatype-dev@lists.helixcommunity.org
>
>>>Subject: RE: [datatype-dev] CR: Thumbnail generation improvement updates
>
>>>
>
>>>
>
>>>
>
>>>+                  UCHAR* pTmpBuffer = NULL;
>
>>>
>
>>>+                  int iSize = WIDTHBYTES(m_ulOutputWidth * 12) * m_ulOutputHeight;
>
>>>
>
>>>+                  pTmpBuffer = new UCHAR[iSize];
>
>>>
>
>>>+                  memset(pTmpBuffer, 0, iSize);
>
>>>
>
>>>+
>
>>>
>
>>>+                  // Resizing the video frame first
>
>>>
>
>>>+                  retVal = ResizeYUV420(pData, pTmpBuffer, m_ulInputWidth, m_ulInputHeight,
>
>>>m_ulOutputWidth, m_ulOutputHeight, 0);
>
>>>
>
>>>+
>
>>>
>
>>>
>
>>>
>
>>>This code assumes that we are converting from I420, which may
>
>>>
>
>>>not always be true. So here:
>
>>>
>
>>>
>
>>>
>
>>>+                  int iSize = WIDTHBYTES(m_ulOutputWidth * 12) * m_ulOutputHeight;
>
>>>
>
>>>
>
>>>
>
>>>we cannot assume that bitsPerPixel is 12.
>
>>>
>
>>>[Hayoung] Okay. I'll replace it with pHeader->biBitCount value.
>
>>>
>
>>>
>
>>>
>
>>>Also, the ResizeYUV420 function I believe currently only
>
>>>
>
>>>works on I420 frames. If so, then we need to have some sort
>
>>>
>
>>>of graceful fallback when the renderer blts something other
>
>>>
>
>>>than I420.
>
>>>
>
>>>[Hayoung] I see. Then I¡¯ll proceed it to return HXR_FAIL error when it comes with other than I420
>
>>>input frame.
>
>>>
>
>>>
>
>>>
>
>>>Also, when we resize, are you correctly setting the "Width"
>
>>>
>
>>>and "Height" in the outgoing stream header to the new width
>
>>>
>
>>>and height?
>
>>>
>
>>>[Hayoung] Correct. It sets the width and height value as the resized one.
>
>>>
>
>>>
>
>>>
>
>>>Eric
>
>>>
>
>>>
>
>>>
>
>>>=======================================
>
>>>
>
>>>Eric Hyche (ehyche@real.com)
>
>>>
>
>>>Principal Engineer
>
>>>
>
>>>RealNetworks, Inc.
>
>>>
>
>>>
>
>>>
>
>>>
>
>>>
>
>>>>-----Original Message-----
>
>>>
>
>>>>From: datatype-dev-bounces@helixcommunity.org [mailto:datatype-dev-bounces@helixcommunity.org] On
>
>>>
>
>>>>Behalf Of ???(Hayoung Choi)
>
>>>
>
>>>>Sent: Wednesday, October 08, 2008 8:13 AM
>
>>>
>
>>>>To: datatype-dev@lists.helixcommunity.org
>
>>>
>
>>>>Subject: RE: [datatype-dev] CR: Thumbnail generation improvement updates
>
>>>
>
>>>>
>
>>>
>
>>>>The attached file is changed. Please review this file.
>
>>>
>
>>>>
>
>>>
>
>>>>Thanks
>
>>>
>
>>>>
>
>>>
>
>>>>________________________________
>
>>>
>
>>>>
>
>>>
>
>>>>From: datatype-dev-bounces@helixcommunity.org ÀÌ(°¡) ´ÙÀ½ »ç¶÷ ´ë½Å º¸³¿ ÃÖÇÏ¿µ(Hayoung Choi)
>
>>>
>
>>>>Sent: 2008-10-08 (¼ö) ¿ÀÈÄ 6:26
>
>>>
>
>>>>To: datatype-dev@lists.helixcommunity.org
>
>>>
>
>>>>Subject: [datatype-dev] CR: Thumbnail generation improvement updates
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>Hi All,
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>I¡¯ve updated this thumbnail CR below.
>
>>>
>
>>>>
>
>>>
>
>>>>The resizing part in OnVideoBlt() has been changed, the others are all same as last CR updated.
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>Please review this.
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>---
>
>>>
>
>>>>
>
>>>
>
>>>>Modified by: Hayoung Choi (hayoung.choi@ap.real.com)
>
>>>
>
>>>>
>
>>>
>
>>>>Date: October 8, 2008
>
>>>
>
>>>>
>
>>>
>
>>>>Project: Thumbnail generation improvement updates
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>Synopsis:
>
>>>
>
>>>>
>
>>>
>
>>>>=============
>
>>>
>
>>>>
>
>>>
>
>>>>This CR enables to convert color format in video decoder with the option ¡°Color Convert¡± (-CC) and
>
>>>PNG
>
>>>
>
>>>>file writer can also change the color format to RGB24 automatically without any option. Also
>
>>resizing
>
>>>
>
>>>>functionality on a decoded video frame is being possible with the option "DecodedVideoResizeWidth"
>
>>(-
>
>>>
>
>>>>DVRW) and "DecodedVideoResizeHeight" (-DVRH). Another, the option ¡°ProcessDecodedTimeUnits¡± (-PDTU)
>
>>>
>
>>>>which limits the maximum decoding counts and "DecodedVideoResizePreserveAspect" (-DVRPA) which
>
>>>adjusts
>
>>>
>
>>>>the aspect ratio is added into video decoder.
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>Overview:
>
>>>
>
>>>>
>
>>>
>
>>>>=============
>
>>>
>
>>>>
>
>>>
>
>>>>1) The color conversion codes are added into the video decoder, so now it is possible to convert
>
>>>color
>
>>>
>
>>>>format has been set by the ¡°ColorConvert¡± (-CC) option in video decoder. In case of writing PNG
>
>>>
>
>>>>thumbnails, if a user didn¡¯t specify any ¡°ColorConvert¡± option, then PNG file writer converts
>
>>>incoming
>
>>>
>
>>>>color format to RGB format automatically.
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>dtdrive.exe +u +f -DV 1 -CC RGB32 -MOF 5 -W videotest.png videotest.rm    // Color conversion will
>
>>be
>
>>>
>
>>>>done in video decoder
>
>>>
>
>>>>
>
>>>
>
>>>>dtdrive.exe +u +f -DV 1 -MOF 5 -W videotest.png videotest.rm              // Color conversion will
>
>>be
>
>>>
>
>>>>done in PNG file writer inside
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>2) Adding ¡°ProcessDecodedTimeUnits¡± option
>
>>>
>
>>>>
>
>>>
>
>>>>The ¡°ProcessDecodedTimeUnits¡± (-PDTU) option means that if an accumulated count of decoded frames
>
>>>
>
>>>>reach the value has been set by the ¡°ProcessDecodedTimeUnits¡± option (-PDTU), then the video
>decoder
>
>>>
>
>>>>calls OnStreamDone() regardless of remaining packet streams. It results in no more decoding.
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>> dtdrive.exe +u +f -DV 1 -PDTU 5 -MOF 10 -W videotest.png videotest.rm
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>3) Resizing decoded video width and height and adding "DecodedVideoResizePreserveAspect" option
>
>>>
>
>>>>
>
>>>
>
>>>>The "DecodedVideoResizeWidth" (-DVRW) and "DecodedVideoResizeHeight" (-DVRH) option means that it
>
>>>
>
>>>>makes the decoded video frame size as the size what you want to resize the width and height.
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>dtdrive.exe +u +f -DV 1 -MOF 5 -DVRW 160 -DVRH 90 -W videotest.png videotest.rm
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>The "DecodedVideoResizePreserveAspect" (-DVRPA) option means that it adjusts the aspect ratio of
>
>>>
>
>>>>resized video. The default value of this is 1, always do adjusting the aspect ratio of source video
>
>>>
>
>>>>frame, but if the zero value has been set by the option, it¡¯ll do not adjusting as the source
>aspect
>
>>>
>
>>>>ratio.
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>> dtdrive.exe +u +f -DV 1 -MOF 5 -DVRW 80 -DVRH 120 -DVRPA -W videotest.png videotest.rm
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>Files Modified:
>
>>>
>
>>>>
>
>>>
>
>>>>=============
>
>>>
>
>>>>
>
>>>
>
>>>>[File 1] - datatype/image/png/filewriter/Umakefil
>
>>>
>
>>>>
>
>>>
>
>>>>[File 2] - datatype/image/png/filewriter/pngwrtr.cpp
>
>>>
>
>>>>
>
>>>
>
>>>>[File 3] - datatype/image/png/filewriter/pngwrtr.h
>
>>>
>
>>>>
>
>>>
>
>>>>[File 4] - datatype/tools/dtdriver/apps/dtdrive/main.cpp
>
>>>
>
>>>>
>
>>>
>
>>>>[File 5] - datatype/tools/dtdriver/engine/ffdriver.cpp
>
>>>
>
>>>>
>
>>>
>
>>>>[File 6] - datatype/tools/dtdriver/engine/pub/ffdriver.h
>
>>>
>
>>>>
>
>>>
>
>>>>[File 7] - datatype/tools/dtdriver/decoder/video/vdecoder.cpp
>
>>>
>
>>>>
>
>>>
>
>>>>[File 8] - datatype/tools/dtdriver/decoder/video/vdecoder.h
>
>>>
>
>>>>
>
>>>
>
>>>>[File 9] - datatype/tools/dtdriver/decoder/video/Umakefil
>
>>>
>
>>>>
>
>>>
>
>>>>[File 10] - build/bif-cvs/helix/common/build/BIF/helix.bif
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>=============
>
>>>
>
>>>>
>
>>>
>
>>>>Image Size and Heap Use impact (Client -Only):
>
>>>
>
>>>>
>
>>>
>
>>>>NA
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>=============
>
>>>
>
>>>>
>
>>>
>
>>>>Platforms and Profiles Affected:
>
>>>
>
>>>>
>
>>>
>
>>>>x86 Windows XP professional SP3 (Intel Core(TM)2 T5600 CPU 1.83GHz)
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>=============
>
>>>
>
>>>>
>
>>>
>
>>>>Distribution Libraries Affected:
>
>>>
>
>>>>
>
>>>
>
>>>>pngwrtr.dll
>
>>>
>
>>>>
>
>>>
>
>>>>dtdrive.lib
>
>>>
>
>>>>
>
>>>
>
>>>>dtdrengine.lib
>
>>>
>
>>>>
>
>>>
>
>>>>dtdrviddec.lib
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>=============
>
>>>
>
>>>>
>
>>>
>
>>>>Distribution library impact and planned action:
>
>>>
>
>>>>
>
>>>
>
>>>>NA
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>=============
>
>>>
>
>>>>
>
>>>
>
>>>>Platforms and Profiles Build Verified:
>
>>>
>
>>>>
>
>>>
>
>>>>System id: win32-i386-vc7
>
>>>
>
>>>>
>
>>>
>
>>>>x86 Windows XP professional SP3 (Intel Core(TM)2 T5600 CPU 1.83GHz)
>
>>>
>
>>>>
>
>>>
>
>>>>Microsoft Compiler 13.10.6030
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>=============
>
>>>
>
>>>>
>
>>>
>
>>>>Platforms and Profiles Functionality verified:
>
>>>
>
>>>>
>
>>>
>
>>>>x86 Windows XP professional SP3 (Intel Core(TM)2 T5600 CPU 1.83GHz)
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>Branch: HEAD and Atlas310
>
>>>
>
>>>>
>
>>>
>
>>>>Target: dtdrive
>
>>>
>
>>>>
>
>>>
>
>>>>Profile: helix-client-all-defines
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>=============
>
>>>
>
>>>>
>
>>>
>
>>>>Copyright assignment:
>
>>>
>
>>>>
>
>>>
>
>>>>I am a RealNetworks employee or contractor
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>
>
>>>
>
>>>>=============
>
>>>
>
>>>>
>
>>>
>
>>>>QA Instructions:
>
>>>
>
>>>>
>
>>>
>
>>>>NA
>
>>>
>
>>>
>
>>>
>
>>>
>
>>>
>
>>>
>
>>>
>
>>>
>
>>
>
>>
>
>
>
>
>
>
>
>



_______________________________________________
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