[prev in list] [next in list] [prev in thread] [next in thread]
List: helix-datatype-dev
Subject: Re: [datatype-dev] CR-Client: add packet merge sort source
From: Milko Boic <milko () real ! com>
Date: 2007-07-10 17:40:09
Message-ID: 6.2.1.2.2.20070710101849.02fdfb70 () mailone ! real ! com
[Download RAW message or body]
Looks good - few remarks of remarks
- CHXMergeSortSourceHandler::Terminate should probably not flush the queued
packets.
In normal operation, StreamDone (for each stream) occurs prior to
termination which will result in proper flushing of the packets. When
Terminate is invoked, the idea is to close down ASAP.
- Tying the processing pipeline position index of the
CHXMergeSortSourceHandler to the name of the properties for the merge
sorter within the merge sorter itself would be a problematic conceptual
binding. As implemented, the "position" is really the
MergeSortSourceHandler type instead of having direct correlation to the
source handler position - which is appropriate. I would suggest making
this explicit and thus clear by changing the "ulPosition " to
"eMergeSorterType" in CHXMergeSortSourceHandler(IUnknown* pContext, UINT32
ulPosition = HXDTDR_MERGE_SORT_POSITION_UNSPECIFIED); and providing
appropriate enum definition.
- To the enum above, I would also add MergeSorterTypeGeneric that could be
placed anywhere in the pipeline purely based on its index.
- Below does not look correct to me since decoder index can be custom set
by the user to any position in the chain and since MERGESORTDEC_OPTION_NAME
is just 0/1 boolean and not an index - attempt to use it as index will
result in incorrect positioning when ulDecodePosition != 1 and
ulPostDecodeSortPosition == 1.
+ // If ulDecodePosition is set to 1, then we will by
+ // default set ulPostDecodeSortPosition to 1. That means
+ // that if we are decoding then by default we will do
+ // a post-decode merge sort.
+ ulPostDecodeSortPosition = ulDecodePosition;
+ // However, even if we *are* decoding and by default do
+ // a post-decode merge sort, setting the MERGESORTDEC_OPTION_NAME
+ // to 0 can still disable the post-decode merge sort.
+ pOptions->GetPropertyULONG32(MERGESORTDEC_OPTION_NAME,
ulPostDecodeSortPosition);
Milko
At 06:29 AM 7/10/2007, Eric Hyche wrote:
>Description
>---------------------------------------
>When using dtdrive as a transcoding engine, the decoded
>packets are passed from the renderer as soon as they
>are produced. Therefore, there usually is a burst of
>packets from one renderer followed by a burst of packets
>from the other renderer. So the packets within a stream
>are delivered in timestamp order, but packets between streams
>are not sorted in timestamp order. Most encoders
>can handle a certain degree of lack of sync between
>audio and video input streams, but when this lack of sync
>extends too far, they throw an error.
>
>To deal with this issue, we introduce a new merge sort
>source handler object. This merge sort source handler
>can be inserted at any point in the source handler stack:
>after the fileformat, after the decoder, or after the
>encoder. When the merge sort source handler is created,
>the user tells it what position it is in.
>
>Based on what position it is in, this soure handler
>will look for its configuration parameters in the options.
>For instance, if it is inserted right after the decoder,
>then it will look for the following configuration
>parameters in the options:
>
>MergeSortMaxQueueDepthDecode - max queue depth allowed
>MergeSortMaxQueueTimespanDecode - max queue time range allowed
>MergeSortMaxQueueBytesDecode - max queue size in bytes
>
>If inserted after the fileformat, then replace "Decode" with
>"FileFormat" in the above options. If inserted after the encoder,
>then replace "Decode" with "Encode" above.
>
>By default when decoding, the merge sort source handler
>is inserted after the decoder. However, this insertion
>can be disabled by setting the "MergeSortDecode" option to 0.
>Command-line options in the dtdrive.exe app were added to
>enable the "MergeSortDecode" option.
>
>This merge sort source handler makes use of the
>CStreamMergeSorter class in datatype/common/filewriter. The
>only changes to this class are to allow for a maximum queue
>size in bytes and an accessor function to get the number
>of streams.
>
>Although this source handler is generic and can be inserted
>anywhere in the source handler stack, currently the only
>place that is enabled in the source handler stack factory
>that is enabled is post-decode, since that is the place
>that we have an immediate need for it.
>
>Files Added
>------------------------------------
>datatype/tools/dtdriver/engine/merge_sort_src_handler.cpp
>datatype/tools/dtdriver/engine/pub/merge_sort_src_handler.h
>
>Files Modified
>------------------------------------
>datatype/common/filewriter/cstrmsrt.cpp
>datatype/common/filewriter/pub/cstrmsrt.h
>datatype/tools/dtdriver/apps/dtdrive/Umakefil
>datatype/tools/dtdriver/apps/dtdrive/main.cpp
>datatype/tools/dtdriver/engine/Umakefil
>datatype/tools/dtdriver/engine/csrchdlrstackfact.cpp
>datatype/tools/dtdriver/engine/csrchdlrstackfact.h
>datatype/tools/dtdriver/engine/pub/ffdriver.h
>helix.bif
>
>Branches
>------------------------------------
>HEAD, 150cay, 203cay (maybe), 204cay (maybe)
>
>Testing
>------------------------------------
>Ran "dtdrive +u -DA 1 -DV 1 kpax.rm" and put the output
>into withsort.txt. Ran "dtdrive +u -DA 1 -DV 1 -PDS 0 kpax.rm"
>and put the output into nosort.txt. These files are attached.
>
>=============================================
>Eric Hyche (ehyche@real.com)
>Technical Lead
>RealNetworks, Inc.
>
>
>
>
>
>
>_______________________________________________
>Datatype-dev mailing list
>Datatype-dev@helixcommunity.org
>http://lists.helixcommunity.org/mailman/listinfo/datatype-dev
_______________________________________________
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